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

test(integration): differentiate between integration and end-to-end tests #3351

Merged
merged 5 commits into from
Jun 22, 2024

Conversation

travi
Copy link
Member

@travi travi commented Jun 14, 2024

"integration" tests can be ambiguous to define, so this PR is a step toward drawing better boundaries between our suite of isolated unit/micro tests and larger scoped tests that integrate more parts of the system.

the tests against index.js are larger scoped than the rest of the test files that target specific units of the project. they are more targeted at testing through the public interface and cover more of the full lifecycle, but stop short of doing so in the context of simulating a real git repository host, api, and npm registry. this makes them distinct from both the other unit tests and what i've renamed here to e2e (short for end-to-end) tests.

while "integration" is still potentially a bit ambiguous, it at least aligns with what i've found to be a valuable scope in other projects. i like "integration" tests to refer to tests that interact through the public interface and exercise the whole project up to the point where it reaches out to external systems. open to feedback if others have additional thoughts

Note

i'm not confident that it is valuable to have all three of these categories in this project long term.

i could see converting some of the now-integration tests to the e2e category over time. alternatively, maybe more of the scenarios covered in e2e make sense with a smaller scope within the "integration" category and e2e becomes more specifically targeted at protecting specifically against external integration regressions rather than verifying behaviors. this likely also becomes influenced by efforts to eventually extract some of the core behavior further away from the composition with a default plugins config

for now, this is just to better recognize the categories that already exist in the project

Caution

i think it is worth integrating this with a normal merge rather than a squash because it will help git better keep track of some of the file renames that happened as part of this change

@travi travi marked this pull request as ready for review June 22, 2024 04:24
@travi
Copy link
Member Author

travi commented Jun 22, 2024

i think i finally resolved the instability in the e2e tests as well. it seems it was related to concurrency somehow, so running the tests serially appears to resolve the problems. I still dont love running things in serial and think it suggests test smells, but i dislike it way less than instable verification.

i think it is worth merging this since it is a big improvement from the current state, but i'd like to see us resove the concurrency problems and reintroduce running in parallel over time.

@travi travi requested a review from a team June 22, 2024 04:34
Copy link
Member

@gr2m gr2m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't review in detail, but will do after the fact. I want to unblock it though as current ci is flaky.

I'm looking forward to dive into your changes in detail!

@travi travi merged commit 175c0c9 into master Jun 22, 2024
8 checks passed
@travi travi deleted the integration-tests branch June 22, 2024 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants