-
Notifications
You must be signed in to change notification settings - Fork 2
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
test[e2e]: add more components to create-flow test #3630
Conversation
Removed vultr server and associated DNS entries |
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.
This is looking great 🙌
I'd recommend trying out the Fixture pattern Playwright uses - this would like remove a lot of the boilerplate of passing around page
and locationNode
and allow us to better model our pages.
There are a lot of input boxes identified by data-placeholder rather than placeholder which makes it harder to grab them in tests. Is there a way round this/can they be changed from data-placeholder to a normal placeholder attribute?
Great question - I'm actually not sure where data-placeholder is coming from (maybe something internal to MUI?). I don't know of any reason we can't apply a placeholder attribute where needed. FYI - we should be migrating towards labels for accessibility on our inputs over placeholders. Currently the Editor is not audited, just the public interface, but this is one of many known improvements we'd need to implement.
api.planx.uk/.husky/pre-commit
Outdated
cd ../e2e | ||
pnpm dlx lint-staged |
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've found that I've needed to run pnpm run lint:fix
on every commit from the e2e
directory so hopefully this fixes that
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 not sure this is the right solution - this seems to be running e2e linting each time a commit is made in the api directory. There should be a pre-commit file in the e2e directory which does this (clearly it's not working though!)
I think we need to actually install lint-staged in the root (docs) and then we can have a per-project husky working as expected?
Thanks @DafyddLlyr - I'm trying this out in the next PR: https://github.com/theopensystemslab/planx-new/pull/3683/files#diff-1307319267989b018c8a9fe05fcaf699ea2d8459c020391fea5868e32719e8dfR108-R127. So far it only looks like I've moved the complexity around a bit but I'm hoping to find a way to "fixturize" this and make it useful for the other tests in create-flow.spec.ts |
In this PR
✅ I think this is the end of the 'straightforward' components, at least from the adding to a flow side.
⚔️ There are other components still to cover, but are difficult or skipped here because:
Still to do 📝
❓ Questions
data-placeholder
rather thanplaceholder
which makes it harder to grab them in tests. Is there a way round this/can they be changed from data-placeholder to a normal placeholder attribute?