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
split unit and integration tests for blueprint #6473
Conversation
these look kinda unit testy, can you share your thoughts? |
My thoughts are unit tests test parts of a blueprint model, with dependencies like the filesystem mocked. Integration tests involve an actual (albeit fixture) blueprint and test all the pieces together including filesystem artifacts. My reasoning in a nutshell is: When I want to write a new test, I know whether I want to test the unit or test the whole blueprint, but when go to find where to put the test, I have to read through the entire test file of mixed unit/integration tests to find where to put my test. |
Some thoughts:
After reading through blueprint's "unit" tests earlier, I agree some are integration tests. But many are merely interacting with mocks and fixtures data, which suggests they are unit tests. For those interacting with very much discrete high level un-mocked units, they should be considered integration tests. |
☔ The latest upstream changes (presumably #6475) made this pull request unmergeable. Please resolve the merge conflicts. |
@kellyselden ping? |
I think the FS is a huge variant. Wether it's Windows vs Unix, sudo vs non sudo, local vs CI, I think there are a number of reasons why a unit test would pass because you are calling all the right
Only if you rely solely on unit tests.
I agree mocks imply unit test, but fixtures imply integration. For example, if my unit I'm testing does a string replace, but the line endings in the fixture files are messing with my test, it's no longer a unit test, but an integration test IMO. |
Unit tests, that interact first-class with FS, can hit fixtures. If they are indirectly depending on the FS, it should be mocked. Im fine with merging this, |
8c77529
to
e01af2e
Compare
e01af2e
to
d801b65
Compare
@homu r+ |
📌 Commit d801b65 has been approved by |
⚡ Test exempted - status |
split unit and integration tests for blueprint The blueprint-test is huge and has both unit and integration tests. This splits them up. Should probably be applied across the board.
The blueprint-test is huge and has both unit and integration tests. This splits them up. Should probably be applied across the board.