-
Notifications
You must be signed in to change notification settings - Fork 365
[WIP] Cypress module resolution #9631
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[WIP] Cypress module resolution #9631
Conversation
0438dc6 to
c1c2953
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - not sure on the name @cypress-support since it make it appear like those support files come from cypress as opposed to being built by us. Even so, that's very minor, so i'm good to merge this...if we come up with a better name, we can change in a follow up. Was thinking @cypress-miq-support, but maybe that's a bit wordy.
|
I think schedule form merge #9549 caused a conflict. I'll still take a look. Rebase when you can. |
cypress.config.js
Outdated
| const webpackConfigOptions = { | ||
| resolve: { | ||
| alias: { | ||
| '@cypress-support': resolve(__dirname, 'cypress/support'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I think I prefer adding the top-level cypress directory as '@cypress-dir' so the imports can be:
@cypress-dir/support/assertions/assertion_constants.js
@cypress-dir/support/assertions/expect_alerts.js
The other benefit is we can do other directories in @cypress-dir:
@cypress-dir/fixtures/empty_notifications.json
If we add factory bot commands to create objects you need for tests, they'll likely live in a directory that is a sibling to fixtures and support directories.
I guess we can add more directories to this list but I think a single toplevel cypress directory might enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So basically
| '@cypress-support': resolve(__dirname, 'cypress/support'), | |
| '@cypress-dir': resolve(__dirname, 'cypress'), |
I'm surprised cypress doesn't have that built-in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised cypress doesn't have that built-in
i.e. https://docs.cypress.io/app/references/configuration#Folders--Files - would be nice to do something like Cypress.config('cypressFolder') if something like that existed
closest I could find is supportFile under https://docs.cypress.io/app/references/configuration#e2e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I tried looking too... I don't get it. seeing ../../ with a different number of directories melts my brain 🫠 .
The idea came to me when looking at cypress-on-rails... I had to specify a different install_folder because it's in manageiq ui classic's cypress directory, not in the rails app, which is non-standard. https://github.com/search?q=repo%3Ashakacode%2Fcypress-playwright-on-rails%20install_folder&type=code
I'm doing this in my cypress-on-rails POC...
CypressOnRails.configure do |c|
c.install_folder = ManageIQ::UI::Classic::Engine.root.join("cypress/e2e")
Of course, this is in the ruby side but the need is the same: a convenient way to get the cypress directory to standardize how to "find" code relative to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to top-level directory
I agree, but offered a slightly different idea to use the top-level cypress directory as @cypress-dir as that will allow us to load fixtures, vcr cassettes, support files, custom commands, etc. specific to cypress. |
c1c2953 to
03dfd4c
Compare
|
Looks like it has a conflict. My only concern is the dependencies we're pulling in. It seems like a lot. |
fcb55a2 to
171866a
Compare
140a052 to
e33c953
Compare
39ebd9a to
fc1a45a
Compare
fc1a45a to
21f4253
Compare
|
CI is now using Node.js v20.19.5, and it's failing with: @miq-bot add-label blocked |
Follow up work from this PR:
PR to add module resolution support in Cypress using @cypress/webpack-preprocessor because relative paths can be messier to handle going forward as mentioned here:
e.g.
flashClassMapis something that we need across the test forexpect_flashcommand. So, that can be imported using a module resolver alias that we define:Instead of a relative import:
import { flashClassMap } from '../../../../support/assertions/assertion_constants';we can do:
import { flashClassMap } from '@cypress-support/assertions/assertion_constants.js';here
@cypress-supportis the alias that we define to get intocypress/supportdirectory.Why not set Cypress aliases directly in the webpack-config
resolve.aliasand use them in tests without relying on preprocessor?Cypress, by default, uses its own internal preprocessor to bundle the test files. This preprocessor handles how the JavaScript and other assets are resolved and compiled before tests run in the browser. To apply a custom
resolve.aliasconfiguration to the tests, we need a mechanism to integrate that configuration with Cypress's preprocessor. This is precisely what@cypress/webpack-preprocessor facilitates. It allows to pass specific Webpack options (includingresolve.alias) to Cypress's internal bundling process. Without the preprocessor, Cypress's default bundling mechanism would not be aware of custom aliases defined in webpack-config(our case shared.js) and would fail to resolve modules using those aliases, leading to errors like "Module not found."Note:
We're using preprocessor version "2.0.1" at the moment. Once Webpack is upgraded, we should move to the latest version. Although it's compatible with Webpack 4+ and Babel 7+, we'll temporarily stay on an older version due to some errors. Also we're currently using a separate configuration object for Cypress instead of relying on the webpack config in shared.js, due to some existing errors. Hopefully, these issues will be resolved after upgrading to Webpack 5, at which point we can move the Cypress-specific aliases back into the main config.
@miq-bot add-label cypress
@miq-bot add-label enhancement
@miq-bot assign @jrafanie