Prettier and ESLint are currently not enforced on Cypress tests. This causes issues with MRs when some people's IDEs will automatically format their Cypress tests but some peoples don't.
A solution to this would be to move the Cypress tests out of the /tests directory and into the /ui folder where it is already being enforced AND it also would not conflict with Drupal core's formatting rules. When trying to expand eslint enforcement to the Cypress file, it runs into a conflict with the prettier used by core.
Move Cypress tests to the /ui folder
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
hooroomooComment #3
wim leersComment #4
wim leers#3460760: CI: improve UI-related CI jobs:`UI eslint` obscures TypeScript Compiler errors, `npm run build` runs too late was the last issue that touched this area in a significant way — you'll probably find helpful pointers there :)
Comment #7
omkar-pd commentedComment #8
wim leersLet's land this!
Comment #9
balintbrewsLet's rename the directory to
tests, please:ui/cypress→ui/tests. That is more conventional, and it's how people would look for tests. I think the current structure under that folder is fine, I don't see the need to have a subdirectory namedcypress.Also, when I tested the changes locally, I got many ESLint errors and required formatting changes when running
npm run lint:eslintandnpm run format. Let's make sure those are all gone before we merge the MR, otherwise we will break all MRs that merge in upstream code.Comment #10
hooroomooComment #11
hooroomooWill continue this
Comment #12
hooroomooPostponing this to after #3475759: Implement a different approach to the iFrame preview interactions is in since that touches many many Cypress files and would make sense to fix the errors the eslint-cypress plugin surfaces after that's in to avoid extra work.
Comment #13
jessebaker commented#3475759: Implement a different approach to the iFrame preview interactions has landed!
Comment #14
hooroomooComment #15
hooroomooA lot of the cypress eslint errors were for this rule: cypress/unsafe-to-chain-command. https://github.com/cypress-io/eslint-plugin-cypress/blob/master/docs/rules/unsafe-to-chain-command.md
This was good reading since the incorrect way seems very intuitive to me but turns out it's not recommended! :
https://docs.cypress.io/app/core-concepts/retry-ability#Actions-should-be-at-the-end-of-chains-not-the-middle
Comment #16
hooroomooComment #17
jessebaker commentedComment #18
hooroomooComment #19
balintbrewsI went through all changes and approved the MR. Thanks for the hard work on this, everyone!
We're blocked on a code owner approval for the changes in
gitlab-ci.yml. I naïvely thought I could temporarily add myself as a code owner, and give the approval, but it didn't work. (Makes sense that it didn't.) I assume that would need to be done upstream. I opened #3483966: Update "DX: CI" section of CODEOWNERS.Comment #22
effulgentsia commentedThanks for opening #3483966: Update "DX: CI" section of CODEOWNERS. That'll be good to resolve. In the meantime, I merged this by temporarily turning off codeowner protection on 0.x. I turned that back on though right after merging this.
Comment #23
wim leersYay, thanks for this thorough improvement, @hooroomoo! 🙏