Skip to content

intermediate-images inherit-label test: make it debuggable#4837

Merged
rhatdan merged 1 commit intocontainers:mainfrom
edsantiago:test_better
Jun 5, 2023
Merged

intermediate-images inherit-label test: make it debuggable#4837
rhatdan merged 1 commit intocontainers:mainfrom
edsantiago:test_better

Conversation

@edsantiago
Copy link
Member

Test is currently very hard to debug on failure. Fix that
by adding unique test descriptors and a little whitespace.

Also, fix a broken/NOP test (copypaste artifact)

Signed-off-by: Ed Santiago santiago@redhat.com

None

Test is currently very hard to debug on failure. Fix that
by adding unique test descriptors and a little whitespace.

Also, fix a broken/NOP test (copypaste artifact)

Signed-off-by: Ed Santiago <santiago@redhat.com>
run_buildah inspect --format '{{ index .Docker.Config.Labels "somefancylabel"}}' imagetwo
expect_output ""

run_buildah inspect --format '{{ index .Docker.Config.Labels "somefancylabel"}}' imagethree
Copy link
Member Author

Choose a reason for hiding this comment

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

SPECIAL NOTE OF THIS CHANGE! github's diff highlighter did not highlight the critical change here: changing imagetwo to imagethree. I am 99.99% certain that this was a copy-paste error. If it is not, please yell loudly now.

Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

tests/helpers.bash says expect_output is deprecated by assert. Now, I'm sure it does not make sense to convert everything to assert, but maybe use a new version when you touch the code.

Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM overall.

@edsantiago
Copy link
Member Author

@kolyshkin point taken, but I was going for ease of review to get this in ASAP (for your #4832, actually, in case it could be helpful in tracking down that weird CI failure).

@rhatdan
Copy link
Member

rhatdan commented Jun 5, 2023

/approve
/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 5, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, rhatdan

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

The pull request process is described here

Details 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

@openshift-ci openshift-ci bot added the approved label Jun 5, 2023
@rhatdan rhatdan merged commit 8543176 into containers:main Jun 5, 2023
@edsantiago edsantiago deleted the test_better branch June 6, 2023 11:37
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants