Skip to content
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

Added a few simple test cases #1041

Merged
merged 4 commits into from
Sep 4, 2023
Merged

Added a few simple test cases #1041

merged 4 commits into from
Sep 4, 2023

Conversation

marcus-oscarsson
Copy link
Member

Just added a few simple test cases, more to come (also feel free to add :) )

@axelboc
Copy link
Collaborator

axelboc commented Sep 1, 2023

Nice!! I'll try to set up cypress-testing-library at some point next week.

@marcus-oscarsson
Copy link
Member Author

@axelboc Thanks Axel that would be cool :). I also wanted to stop by and ask you a few other questions about how to best compose the tests :)

run: |
mxcubeweb-server -r ./test/HardwareObjectsMockup.xml/ --static-folder $(pwd)/ui/build/ -L debug &
- name: Cypress run
uses: cypress-io/github-action@v5
Copy link
Collaborator

@axelboc axelboc Sep 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This action was also installing dependencies. In fact, it's quite the behemoth: it does the caching, the installation, can even run a dev server ... I prefer to make all these steps explicit for maintainability (e.g. so we won't have to remember to configure it properly if we switch to pnpm in the future, for instance).

@marcus-oscarsson
Copy link
Member Author

Well done @axelboc :) thanks !

run: mxcubeweb-server -r ./test/HardwareObjectsMockup.xml/ --static-folder $(pwd)/ui/build/ -L debug &

- name: Run Cypress
run: npm run --prefix ./ui e2e
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not missing cypress though ;) or ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script is called e2e – it runs cypress run. The script cypress runs cypress open, which opens Cypress for local testing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I see, sorry for the confusion

@@ -45,7 +45,7 @@ jobs:
- name: Install dependencies
run: |
pip install -e .
npm install --prefix ./ui
npm install --prefix ./ui --legacy-peer-deps
Copy link
Member Author

@marcus-oscarsson marcus-oscarsson Sep 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--legacy-peer-deps because the node version is old ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just a flag for NPM to disregard peer deps warnings and install any missing peer deps automatically. With pnpm, I would have added an override in package.json. The warning just comes from @testing-library/cypress which does not allow the latest version of Cypress as peer dep — no problem with ignoring this warning if the tests pass.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh, ok

@marcus-oscarsson marcus-oscarsson merged commit 48199ef into develop Sep 4, 2023
8 checks passed
@marcus-oscarsson marcus-oscarsson deleted the mo-test branch September 20, 2023 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants