-
Notifications
You must be signed in to change notification settings - Fork 116
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
Adds Playwright, first tests and linting. #1529
Conversation
This runs in your local database and is noisy! I'd rather run it with a separate database but that might be overkill, I don't know.
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.
@tharsheblows Looks great so far!
I think we could use this setup to run E2E tests locally for now and not worry about the CI integration at this point. If you think that's a good approach, we just need to make sure test:e2e
is not run in GitHub action.
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.
📝 note: @tharsheblows Let's also make sure .distignore
is kept up to date.
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.
Thank you! Ack, this is one of the reason I love code reviews, I completely forgot about that.
This renames tests/tests to tests/phpunit for clarity.
@delawski I've updated this to not run the e2e tests in the GH action by renaming it and added the files to |
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.
Great idea about organizing tests in the tests
folder the way you did it 👍
It looks like one of the tests is failing for me. Is it working okay on your local?
# Conflicts: # package-lock.json # package.json
@tharsheblows I'm sorry about the dependencies update train when you were trying to merge it to I resolved the conflicts now and will merge as soon as the CI checks are complete. |
Fixes #1528.
This runs in your local database and is noisy! I'd rather run it with a separate database but that might be overkill, I don't know. Currently it causes the lint action to fail as there's not the executables installed.