-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
test(e2e): Run e2e selectively #17077
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
base: develop
Are you sure you want to change the base?
Conversation
|
||
// Check if only test application files were changed | ||
const changedTestApps = new Set<string>(); | ||
const hasSharedCodeChanges = changedFiles.some(file => { |
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.
IMHO it's a bit weird/unexpected to use some
and then collect stuff there 🤔
I think it would be easier to read/follow to extract this into a method like:
function getChangedTestApps(): false | Set<string>
And then do
const changedApps = getChangedTestApps();
if (!changedApps) {
return testApplication;
}
// return the changed apps only...
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 can move that into a function but would probably still make use of some
so the loop stop whenever we have a match?
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.
refactored a bit and replaced some with a basic for loop, hope that's more readable!
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.
Bug: Shared Code Changes Ignored in Test Coverage
Files placed directly in dev-packages/e2e-tests/test-applications/
(not within a specific test app subdirectory) are incorrectly ignored by the getChangedTestApps
function. This occurs because the slashIndex > 0
condition prevents processing files where indexOf('/')
returns -1. These files, which are effectively shared code, should trigger a full test run (i.e., getChangedTestApps
should return false
), but their changes are currently missed, leading to incomplete test coverage.
dev-packages/e2e-tests/lib/getTestMatrix.ts#L196-L199
sentry-javascript/dev-packages/e2e-tests/lib/getTestMatrix.ts
Lines 196 to 199 in ff87af2
if (slashIndex > 0) { | |
changedTestApps.add(pathAfterPrefix.slice(0, slashIndex)); | |
} |
Was this report helpful? Give feedback by reacting with 👍 or 👎
This pr changes the detection logic for the e2e tests directory.
Before we always ran every test whenever something changed in this dir.
With this change we check for changed files in the test apps and only run all tests if shared code has been changed.