-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add Playwright with eslint and example login test #1590
Conversation
… feat.add-playwright
… feat.add-playwright
run: | | ||
cp example.env .env | ||
|
||
- name: Install API-service code-dependencies |
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.
Can this step be removed? The docker image should have the required libraries from this step.
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.
@gulfaraz good question. It seemed necessary, because the e2e tests are run from the host with 'npm test', and they in turn use some of the API-service files (e.g. utility.helper.ts) to reset the DB, which in turn needs the 'supertest' package. And without this 'npm install' step on the host instead of within the container, this package was not found.
Do you think this setup should be changed?
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.
It shouldn't be that the installation is needed on the host machine while the service is running inside docker.
I will investigate this before I propose a change.
@Piotrk39 is there a benefit to keep the e2e tests in a separate workflow? I would like a setup where a failure of an end-2-end test will block the version bump in the other workflow. As these workflows are currently independent, the version gets bumped regardless of the e2e tests. FYI @jannisvisser |
@gulfaraz We should add new item to fix that
|
@gulfaraz could we alternatively block merging to master if not all tests succeed? |
Yes, if we can figure out a way to link the other workflow to the version bump step. It also doesn't make sense to me that we repeat certain steps in both workflows. Github actions has a limit so I think it's wise to use this limited resource efficiently. But if the need is there we can absolutely repeat certain steps, this is why I asked if the decision to have a separate workflow is deliberate. |
Describe your changes
Adds Playwright E2E automation tool to IBF-system Github repository, that includes:
ToDo:
Checklist before requesting a review