Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
+49 −1
Open

test(e2e): Run e2e selectively #17077

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0b86c7a
update text matrix for e2e tests
chargome Jul 18, 2025
96e4742
fallback
chargome Jul 18, 2025
d1dfa68
smol cleanup
chargome Jul 18, 2025
ff87af2
Merge branch 'develop' into cg-e2e-test-selectively
chargome Jul 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion dev-packages/e2e-tests/lib/getTestMatrix.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,47 @@ function getAffectedTestApplications(
.map(line => line.trim())
.filter(Boolean);

// If something in e2e tests themselves are changed, just run everything
// If something in e2e tests themselves are changed, check if only test applications were changed
if (affectedProjects.includes('@sentry-internal/e2e-tests')) {
try {
const changedFiles = execSync(
`git diff --name-only ${base}${head ? `..${head}` : ''} -- dev-packages/e2e-tests/`,
{ encoding: 'utf-8' },
)
.toString()
.split('\n')
.map(line => line.trim())
.filter(Boolean);

// Check if only test application files were changed
const changedTestApps = new Set<string>();
const hasSharedCodeChanges = changedFiles.some(file => {
Copy link
Member

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...

Copy link
Member Author

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?

Copy link
Member Author

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!

// Check if the file is within a test application directory
const testAppMatch = file.match(/^dev-packages\/e2e-tests\/test-applications\/([^/]+)\//);
if (testAppMatch?.[1]) {
changedTestApps.add(testAppMatch[1]);
return false;
}
// If it's not in test-applications/, we assume it's shared code
return true;
});

// Shared code was changed, run all tests
if (hasSharedCodeChanges) {
return testApplications;
}

// Only test applications were changed, run selectively
if (changedTestApps.size > 0) {
return testApplications.filter(testApp => changedTestApps.has(testApp));
}
} catch (error) {
// eslint-disable-next-line no-console
console.error('Failed to get changed files, running all tests:', error);
return testApplications;
}

// Fall back to running all tests
return testApplications;
}

Expand Down