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

Update dbt-core dependency of dbt-common for testing #105

Merged
merged 8 commits into from
Apr 24, 2024

Conversation

emmyoop
Copy link
Member

@emmyoop emmyoop commented Apr 12, 2024

resolves #104
Related to initial fix in #68

Description

When we run dbt-core tests we're not using the right sha when we're merging in so a change that breaks core doesn't block the merge.

This leverages a new script from #105 to update the dev-requirements so that tox installs the correct sha.

Checklist

@emmyoop emmyoop marked this pull request as ready for review April 12, 2024 18:29
@emmyoop emmyoop requested a review from a team as a code owner April 12, 2024 18:29
@emmyoop emmyoop added this pull request to the merge queue Apr 16, 2024
@emmyoop emmyoop removed this pull request from the merge queue due to a manual request Apr 16, 2024
@emmyoop emmyoop changed the title debugging all the values Fix merge sha Apr 22, 2024
@emmyoop
Copy link
Member Author

emmyoop commented Apr 22, 2024

From the test I ran:
These are the output shas when running in the merge queue

dbt-core-ref=main
dbt-common-ref=561ec9f361f64895ef7c51598d98474a1314900b
merge queue values
github.event.merge_group.head_sha=561ec9f361f64895ef7c51598d98474a1314900b
github.event.merge_group.head_ref=refs/heads/gh-readonly-queue/main/pr-105-b76d0e655a81873646f9d32a9e4df170cedcafa8
github.event.merge_group.base_sha=b76d0e655a81873646f9d32a9e4df170cedcafa8
github.event.merge_group.base_ref=refs/heads/main
github.event.merge_group.head_commit.id=561ec9f361f64895ef7c51598d98474a1314900b
github.event.merge_commit_sha=
GITHUB_SHA=561ec9f361f64895ef7c51598d98474a1314900b
PR values
github.event.pull_request.base.sha=
github.event.pull_request.head.sha=
github.event.pull_request.merge_commit_sha=

These are how they map:

sha of my branch: 274c63b1702fd96fadbaa92c14bc9056658c8526
sha of main at this point: b76d0e655a81873646f9d32a9e4df170cedcafa8
sha of the merge: 561ec9f361f64895ef7c51598d98474a1314900b

We want to use the merge sha so the following should work:

github.event.merge_group.head_sha
github.event.merge_group.head_commit.id
$GITHUB_SHA

We are currently using github.event.merge_group.head_sha here. This should work. Will monitor the next merge with changes to check.

@cla-bot cla-bot bot added the cla:yes label Apr 24, 2024
@codecov-commenter
Copy link

codecov-commenter commented Apr 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.26%. Comparing base (753c03f) to head (6b6bb8e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #105   +/-   ##
=======================================
  Coverage   56.26%   56.26%           
=======================================
  Files          50       50           
  Lines        3082     3082           
=======================================
  Hits         1734     1734           
  Misses       1348     1348           
Flag Coverage Δ
unit 56.26% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@emmyoop emmyoop changed the title Fix merge sha Update dbt-core dependency of dbt-common for testing Apr 24, 2024
@emmyoop emmyoop requested a review from peterallenwebb April 24, 2024 14:49
@emmyoop emmyoop added the Skip Changelog Skips GHA to check for changelog file label Apr 24, 2024
@emmyoop emmyoop added this pull request to the merge queue Apr 24, 2024
Merged via the queue into main with commit 2fef811 Apr 24, 2024
17 checks passed
@emmyoop emmyoop deleted the er/fix-core-merge-queue-testing branch April 24, 2024 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes Skip Changelog Skips GHA to check for changelog file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] [spike] Wrong commit used for core testing
4 participants