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

Integration tests are broken and unit tests are missing #693

Closed
mturley opened this issue Feb 21, 2020 · 4 comments
Closed

Integration tests are broken and unit tests are missing #693

mturley opened this issue Feb 21, 2020 · 4 comments
Labels
icebox General backlog kind/bug Categorizes issue or PR as related to a bug. tech-debt Issues describing technical debt to be refactored when appropriate

Comments

@mturley
Copy link
Collaborator

mturley commented Feb 21, 2020

Running yarn test fails with errors in both the cluster form spec and the storage form spec.

Jest is installed and configured, but no Jest tests exist. I think things like "has buttons disabled when input data is missing" are more easily implemented as Jest unit tests of the individual component, and integration tests should only test the interactions between different components.

Related to #310, but in addition to snapshot tests we should have smaller tests asserting specific things without a snapshot, e.g. we could shallow-render the form with input data missing and assert that the buttons have the isDisabled props set correctly. I'm open to debate how useful snapshot tests really are, anyway :)

@eriknelson eriknelson added kind/bug Categorizes issue or PR as related to a bug. release-1.2.0 Required issue for 1.2 release labels Feb 28, 2020
@eriknelson
Copy link
Contributor

eriknelson commented Feb 28, 2020

This is two separate problems. There's a regression here where our travis CI should be failing every PR and it's not. Separately, the entire project is undertested and this is a known issue. We need to make it a priority to fix travis so that those tests are correctly failing if they are not passing our basic existing tests.

@dymurray dymurray added icebox General backlog tech-debt Issues describing technical debt to be refactored when appropriate and removed release-1.2.0 Required issue for 1.2 release labels Mar 30, 2020
@gildub
Copy link
Collaborator

gildub commented Apr 8, 2020

@mturley, as @eriknelson commented this are 2 separate issues.
Would it be okay to split it. Maybe keep this one for the broken integration tests issue and create another one for sub-snapshot tests?

@eriknelson
Copy link
Contributor

I'd like to close this issue so we can open two new issues that accurately reflect where we are currently. The "integration tests" we a long time ago were really not accomplishing anything and so they sat and rotted. I pulled them and archived them.

We need two issues, one that documents our desired strategy for integration testing, and I have a pretty good idea what this needs to look like in the long run. Separately, as @mturley pointed out, it's time to get some snapshot testing in place where feasible. I'd like to schedule a meeting with the UI team to discuss these, and out of that meeting produce these issues with our documented conclusions. I will schedule something this week, it's going to be a longer discussion than we can handle on a scrum call.

@mturley
Copy link
Collaborator Author

mturley commented Apr 8, 2020

Sounds good, thanks guys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
icebox General backlog kind/bug Categorizes issue or PR as related to a bug. tech-debt Issues describing technical debt to be refactored when appropriate
Projects
None yet
Development

No branches or pull requests

4 participants