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
Move e2e to api tests and unify test command #14733
Conversation
Codecov ReportBase: 58.76% // Head: 58.78% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #14733 +/- ##
==========================================
+ Coverage 58.76% 58.78% +0.02%
==========================================
Files 1340 1341 +1
Lines 32489 32511 +22
Branches 6143 6145 +2
==========================================
+ Hits 19091 19113 +22
Misses 11510 11510
Partials 1888 1888
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Considering that the way the user interacts with Strapi is the API and that's what most of these are testing, I'm not 100% convinced those couldn't still be considered e2e tests that need their descriptions tweaked to read like user stories. But since they are all API related, and they all only test a single operation e2e and none of them test a sequence, I think it's fine to separate them like this. I like that it opens the path for us to create complex e2e tests in the future, including e2e testing the admin UI, etc and to keep those separate from these simple tests of the API. That's something we're lacking. The only comment I have on functionality is that I'm not sure about changing the default from running EE to CE. If CE is used as a default rather than specified, we should output a warning that it's running an incomplete test suite. My earlier concern with the difficulty onboarding contributors wasn't necessarily that EE tests get run by default, just that there wasn't a simple yarn script available to run the tests, and another way to run only CE. Here are a few things I noticed while reviewing but would probably be better as a separate PR. Feel free to ignore and I'll bring them up at a later time:
|
189b914
to
6ae9c6f
Compare
I tthink it's wrong to consider CE tests as an incomplete version of EE tests. Those are 2 different versions and have tests in common but also specific tests each so each run is complete for the specific version. As Strapi is starting in CE by default I think it makes to have the same implicit behavior with tests
Indeed I'm not going to explore this for now but that's a good note |
Ok, I agree with that. |
What does it do?
yarn test:api
and make default tests to be CE and enable EE when detecting a STRAPI_LICENSE env var only.Why is it needed?
Describe the issue you are solving.
How to test it?
Provide information about the environment and the path to verify the behaviour.
Related issue(s)/PR(s)
Let us know if this is related to any issue/pull request