Skip to content

Fix detection of which tests to run when "quick" tests are requested. #539

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

Merged
merged 27 commits into from
Jul 15, 2021

Conversation

jonsimantov
Copy link
Contributor

@jonsimantov jonsimantov commented Jul 14, 2021

Previously, the code was attempting to auto-diff with ${{github.event.pull_request.base.sha}}.

As it turns out, this is incorrect - the base sha in this case points to the current head of main, not the head of main when the branch was created (which is closer to what we want).

Instead, we now do the actual correct thing: query git to find the common ancestor commit of origin/main and origin/[pr branch name], and diff the PR's branch (again from origin) against that common ancestor.

This makes it much more robust to detect which files were actually edited by a PR, even if the base main branch has since been modified (and even if those modifications were partially merged into this branch).

Also in this PR: change to print_matrix_configuration to allow us to treat certain files as being within a given subdirectory when looking through the file list in auto-diff mode. For example, cmake/external/firestore.cmake should be considered under the "firestore" directory.

@google-cla google-cla bot added the cla: yes label Jul 14, 2021
@jonsimantov jonsimantov changed the title Print out what's triggering the tests. Fix detection of which tests to run when "quick" tests are requested. Jul 14, 2021
@jonsimantov jonsimantov added the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. and removed tests-requested: quick Trigger a quick set of integration tests. labels Jul 14, 2021
@github-actions
Copy link

github-actions bot commented Jul 14, 2021

✅  Integration test succeeded!

Requested by @jonsimantov on commit 26054b4
Last updated: Thu Jul 15 01:25 PDT 2021
View integration test log & download artifacts

@jonsimantov jonsimantov added the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@github-actions github-actions bot removed the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@jonsimantov jonsimantov added the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@github-actions github-actions bot removed the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@jonsimantov jonsimantov added the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@github-actions github-actions bot removed the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@jonsimantov jonsimantov added the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@github-actions github-actions bot removed the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@jonsimantov jonsimantov added the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@github-actions github-actions bot removed the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@jonsimantov jonsimantov added the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@github-actions github-actions bot removed the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@jonsimantov jonsimantov added the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@github-actions github-actions bot removed the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@jonsimantov jonsimantov added the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@github-actions github-actions bot removed the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@jonsimantov jonsimantov requested a review from sunmou99 July 14, 2021 21:44
@jonsimantov jonsimantov added the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@github-actions github-actions bot removed the tests-requested: quick Trigger a quick set of integration tests. label Jul 14, 2021
@jonsimantov jonsimantov requested a review from sunmou99 July 14, 2021 23:15
@github-actions github-actions bot added tests: failed This PR's integration tests failed. and removed tests: failed This PR's integration tests failed. labels Jul 15, 2021
Copy link
Contributor

@sunmou99 sunmou99 left a comment

Choose a reason for hiding this comment

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

LGTM.
Thanks for the explanation.

@jonsimantov jonsimantov enabled auto-merge (squash) July 15, 2021 01:21
@jonsimantov jonsimantov added the tests-requested: quick Trigger a quick set of integration tests. label Jul 15, 2021
@github-actions github-actions bot added tests: failed This PR's integration tests failed. and removed tests-requested: quick Trigger a quick set of integration tests. labels Jul 15, 2021
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jul 15, 2021
@jonsimantov
Copy link
Contributor Author

⏳  Integration test in progress...

Requested by @jonsimantov on commit 1b31f93
Last updated: Wed Jul 14 20:47 PDT 2021
View integration test log & download artifacts

Merging anyway, as it's a flake.

Failure log:
BundleTest.LoadBundlesForASecondTimeSkips (1079 ms)
[ RUN ] BundleTest.LoadInvalidBundlesShouldFail
DEBUG: Using Firestore Prod for testing.
/Users/runner/work/firebase-cpp-sdk/firebase-cpp-sdk/ta/firestore/iti/src/bundle_test.cc:230: Failure
Expected equality of these values:
progresses.size()
Which is: 0
1

@jonsimantov jonsimantov removed the tests: failed This PR's integration tests failed. label Jul 15, 2021
@jonsimantov jonsimantov merged commit 26054b4 into main Jul 15, 2021
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: succeeded This PR's integration tests succeeded. labels Jul 15, 2021
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jul 15, 2021
@jonsimantov jonsimantov deleted the bugfix/fix-quick-test-detection branch July 17, 2021 00:48
@firebase firebase locked and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes tests: succeeded This PR's integration tests succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants