fix: make imported @cypress/react working and pass CI #8718
Conversation
|
Thanks for taking the time to open a PR!
|
dmtrKovalenko
changed the base branch from
develop
to
feature/merge-cypress-react-unit-test
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
| command: yarn workspace @cypress/react test:unit | ||
| - run: | ||
| name: Run tests | ||
| command: yarn workspace @cypress/react test |
There was a problem hiding this comment.
doest this run our example applications to test it?
There was a problem hiding this comment.
No, we can discuss it further, but I think that we should not run tests for file-system based examples as a part of CI flow. Because it takes too much time, requires the installation of a giant amount of unrelated dependencies and basically do not gives as much value.
Checking that we can work with a new UI-kit, or state library is useful – but mostly for users. We anyway cannot cover all the frameworks...
Most of repos containing such examples do not build them on each PR. Check next.js, material-ui. Would be much useful better to include builds for different versions of webpack, experimental version of react, etc.
There was a problem hiding this comment.
Ok, but you see how in cypress-react-unit-test you can change something and none of the existing projects would break right since we have lots of unit tests and complete application tests (aside from accidental dependency found in the root)? And if we want to test different webpack versions, it is the same thing?
So this merge skips what the users actually experience, and thus we cannot guarantee that a new version is not breaking something accidentally, so I have to reject this. The example app tests are there for a reason - because they protect our users.
There was a problem hiding this comment.
The example app tests are redundant in most cases. While they are great examples, the security they provide is often redundant. The differences between Happo, Percy, and Applitools are not relevant to the core repository's functionality.
For example, you don't see lodash testing millions of projects. With a thoughtfully crafted CI strategy, we can retain a comfortable level of security. I would suggest a nightly or bi-weekly build against the example repositories to accomplish this. As we are in alpha with a relatively small userbase and do not promise 100% stability, I am comfortable trading ecosystem coverage for speed.
Lastly, we are not going entirely without coverage. When we change @cypress/react, we trigger a full Cypress binary build and dogfood the changes in an e2e way.
| const mainPackageData = require('../package.json') | ||
|
|
||
| function run () { | ||
| const distPackageData = JSON.stringify({ |
There was a problem hiding this comment.
It's an overall useful practice to not publish the whole project but only dist path.
We can consider to also publish package using yarn publish dist in the future.
|
I think Percy issue is related to some |
|
|
JessicaSachs
changed the base branch from
feature/merge-cypress-react-unit-test
to
master
| @@ -737,7 +737,7 @@ module.exports = (on, config) => { | |||
| The Cypress.io Component Testing Team | |||
|
|
|||
| - [Jessica Sachs](https://github.com/jessicasachs) (Current Maintainer, [Vue Test Utils](https://github.com/vuejs/vue-test-utils) Maintainer) | |||
| - [Gleb Bahmutov](https://github.com/bahmutov) (Original Author, Current Maintainer of [@cypress/react](https://github.com/bahmutov/cypress-react-unit-test)) | |||
| - [Gleb Bahmutov](https://github.com/bahmutov) (Original Author, Current Maintainer of [@cypress/react](https://github.com/bahmutov/@cypress/react)) | |||
There was a problem hiding this comment.
Can you check for typos like these?
JessicaSachs
force-pushed
the
fix/make-imported-cypress/react-work
branch
from
e39ffd6
to
c3d8213
Compare
JessicaSachs
force-pushed
the
fix/make-imported-cypress/react-work
branch
from
c9bd2de
to
e37a242
Compare
No description provided.