Skip to content

test/buildah-bud: fix checkout to also handle go.mod replace #26784

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 1 commit into from
Aug 12, 2025

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Aug 8, 2025

There is a rather surprising bug in the current test checkout logic. The
go.mod version parsing never actually consider a go.mod replace for
buildah and always read the main version.

This meant a buildah replace actually is testing the old version with
the new code and that means the new tests are not run leading people in
false belive when testing a buildah vendor that it worked. But then
later it fails when doing the proper update without replace.

To fix this first use go list to parse go.mod which is more robust. Then
first check if there is a replace and then use that repo/version
instead.

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added release-note-none do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 8, 2025
There is a rather surprising bug in the current test checkout logic. The
go.mod version parsing never actually consider a go.mod replace for
buildah and always read the main version.

This meant a buildah replace actually is testing the old version with
the new code and that means the new tests are not run leading people in
false belive when testing a buildah vendor that it worked. But then
later it fails when doing the proper update without replace.

To fix this first use go list to parse go.mod which is more robust. Then
first check if there is a replace and then use that repo/version
instead.

Signed-off-by: Paul Holzinger <[email protected]>
@Luap99 Luap99 force-pushed the buildah-replace-test branch from 8384d6a to d48c724 Compare August 8, 2025 10:43
@Luap99
Copy link
Member Author

Luap99 commented Aug 8, 2025

Ok this works when vendoring a commit from my fork

         + git clone -q https://github.com/Luap99/buildah test-buildah-v1.16.1-0.20250714142734-bef9eac1323c
[+0009s] + git config --global user.email [email protected]
         + git config --global user.name 'Testy McTestface'
         + git config --global --add safe.directory test-buildah-v1.16.1-0.20250714142734-bef9eac1323c
         + git checkout -q bef9eac1323c

https://api.cirrus-ci.com/v1/artifact/task/5431063180214272/html/bud-podman-fedora-42-root-host-sqlite.log.html

Dropping the vendor test commit so we can merge this.

@Luap99 Luap99 force-pushed the buildah-replace-test branch from d48c724 to 9b62438 Compare August 8, 2025 12:08
@Luap99 Luap99 marked this pull request as ready for review August 8, 2025 12:08
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 8, 2025
@Luap99
Copy link
Member Author

Luap99 commented Aug 8, 2025

cc @nalind @TomSweeneyRedHat

Copy link
Member

@Honny1 Honny1 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

openshift-ci bot commented Aug 11, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Honny1, Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@TomSweeneyRedHat
Copy link
Member

LGTM

@TomSweeneyRedHat
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 12, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit acb75ca into containers:main Aug 12, 2025
38 checks passed
@Luap99 Luap99 deleted the buildah-replace-test branch August 14, 2025 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants