Skip to content

hack: run the tooling self-tests in validate-source - #29288

Merged
giuseppe merged 1 commit into
podman-container-tools:mainfrom
vtushar06:run-tooling-self-tests
Aug 13, 2026
Merged

hack: run the tooling self-tests in validate-source#29288
giuseppe merged 1 commit into
podman-container-tools:mainfrom
vtushar06:run-tooling-self-tests

Conversation

@vtushar06

@vtushar06 vtushar06 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

We have a few self-tests sitting next to the scripts they cover, but nothing actually runs them, so they just rot. hack/ci/logformatter.t is broken right now and nobody noticed.

This wires up the three that pass today and only need python3, bash and perl. I ran the target in a clean container to check.

Left out: xref-helpmsgs-manpages.t needs a built podman and docs so it fits validate-binaries better, pr-should-include-tests.t says in its own header that it does not work with the shallow checkout CI uses, and logformatter.t is currently failing.

pr-removes-fixed-skips already runs in validate-source but its test needs Test::Differences, so I added that to the deps.

Does this PR introduce a user-facing change?

None

Copilot AI review requested due to automatic review settings July 25, 2026 16:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@vtushar06

Copy link
Copy Markdown
Contributor Author

@Luap99 this came out of reading logformatter for #28828 - its self-test fails on main and on the branch in #29091, and I only spotted it because I ran the .t by hand. Nothing runs any of these, so I wired up the ones that are safe to run.

@github-actions github-actions Bot added the CI label Jul 25, 2026
@packit-as-a-service

Copy link
Copy Markdown

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

2 similar comments
@packit-as-a-service

Copy link
Copy Markdown

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

@packit-as-a-service

Copy link
Copy Markdown

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

@lsm5

lsm5 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

/packit build
/packit test

@Luap99 Luap99 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Historically I think we never bother to test these as part of the CI.
It was more of help for whoever was writing the code, I don't think it is important to have them actually part of the validate though I guess it does not hurt as long as the tests itself are fast enough

Comment thread Makefile Outdated
@vtushar06
vtushar06 requested a review from Luap99 July 28, 2026 04:29
@vtushar06
vtushar06 requested a balanced review from Copilot August 7, 2026 13:23
@vtushar06

vtushar06 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@lsm5 I went through the failing job and It got killed mid ginkgo run on one of the new cncf-ubuntu-24-96-x86 runners. a rerun would probably just do the same.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread Makefile Outdated
# Not included here:
# hack/xref-helpmsgs-manpages.t needs a built podman and docs, so it belongs
# with validate-binaries instead
# hack/ci/logformatter.t currently failing, see #28828

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I fixed that tests case in my logformatter PR so I think you can add that as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@Luap99 added it, logformatter.t passes now.

@Luap99

Luap99 commented Aug 10, 2026

Copy link
Copy Markdown
Member

also no point in adding an extra commit for adding more tests, just keep them all as part of one commit

@vtushar06
vtushar06 force-pushed the run-tooling-self-tests branch from 51595b3 to 3558741 Compare August 12, 2026 03:29
@ROKUMATE

Copy link
Copy Markdown
Contributor

@vtushar06 in case it helps with the "one commit" ask ... to squash your two commits into one
You can follow this

git reset --soft HEAD~2               # rewinds the last 2 commits, keeps all changes staged
git commit -s                         # write one combined message
git push --force-with-lease

@vtushar06
vtushar06 force-pushed the run-tooling-self-tests branch from 3558741 to 765aef9 Compare August 13, 2026 03:23
@vtushar06

Copy link
Copy Markdown
Contributor Author

rebased, the two red ones were flakes not this.

Signed-off-by: Tushar Verma <tusharmyself06@gmail.com>
@vtushar06
vtushar06 force-pushed the run-tooling-self-tests branch from 765aef9 to 893059a Compare August 13, 2026 03:30

@Luap99 Luap99 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@podman-container-tools/podman-maintainers PTAL

@giuseppe giuseppe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@giuseppe
giuseppe merged commit 486a3ef into podman-container-tools:main Aug 13, 2026
72 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants