-
Notifications
You must be signed in to change notification settings - Fork 151
Fixes/cypress determinism #384
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
Conversation
@jlubken we've discussed in #351 making it so that defaults could be changed via environment variables in the deployment, so that the source wouldn't need to be changed / new releases made just to update defaults. In that universe, having the Cypress tests dependent on the default values in the source code wouldn't get impacted by frequent updates to defaults via the environment I feel like if we want to actually validate that the mechanics of the frontend are working, we can't avoid checking that a specific input change results in a specific output change. That is, without some assertion that would be broken by defaults changing I don't think we can really validate that the frontend is working end-to-end |
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.
I think the population is one of the "stable" parameters even though it changed today. The other test will cause noise when we update parameters.
Incrementing population didn't result in any outputs changing that we could use to validate that all the wiring is flowing. Would it be so bad if any changes that change results without the cypress test getting updated get flagged by the cypress test in the PR? Contributors would just need to update the Cypress test to confirm the result change is intentional. Seems like not too much a drag that has the benefit of guarding against unintentional changes, but I don't work in that end of the app. What are your thoughts on giving that a try? |
I'm OK with that too. |
I found a lot of evidence that
cy.get('...').click({ multiple: true})
is problematic as it tries to click through everything in the page serially while some elements won't be visibleIn this new basic set of test, I assert on a string to wait for first to let the interface initialize, and test a couple specific steppers independently and check their step and effect on results. This seems to be more consistent, though I don't know that we want so many result numbers being tested at this level vs at the model level in pytest. Might be good to test more of the steppers this way too, but this small initial set is enough to ensure the app is running and wired up