Skip to content

Catch errors in tests of the kitchen sink image - #495

Merged
julienp merged 3 commits into
mainfrom
julienp/fail-test
Jul 17, 2025
Merged

Catch errors in tests of the kitchen sink image#495
julienp merged 3 commits into
mainfrom
julienp/fail-test

Conversation

@julienp

@julienp julienp commented Jun 22, 2025

Copy link
Copy Markdown
Contributor

While looking into pulumi/templates#929 I noticed that the kitchen sink job was not failing https://github.com/pulumi/pulumi-docker-containers/actions/runs/15796277766/job/44529555986#step:16:1064 when it should have run into the same bug. The tests actually do fail, but our setup hides that from CI.

To fix this we run a matrix on the variant: normal and non root.

Also fixes the image name for the nonroot variant.

@julienp julienp added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Jun 22, 2025
@julienp
julienp marked this pull request as ready for review June 23, 2025 07:37
@julienp
julienp requested a review from a team as a code owner June 23, 2025 07:37
Comment thread .github/workflows/ci.yml
@@ -163,7 +163,8 @@ jobs:
run: |

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.

Hrm I guess we didn't catch the errors because of the piping? Not sure why pipefail doesn't fix that. I wonder though, should these just be two test targets, "Tests root" and "Tests nonroot"? Then we don't need any piping to change the output, and less shell magic to wait for processes etc.

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.

The reason for the backgrounding is that we want the 2 tests to run in parallel. I guess we could just matrix it out.

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.

Ah I remember the reason why I didn't go with a matrix, it means we have to build the image twice. Oh well, it will happen in parallel, so it should be fine.

@julienp
julienp force-pushed the julienp/fail-test branch from ad6aebd to ba2a173 Compare June 23, 2025 18:25
@julienp julienp closed this Jun 23, 2025
@julienp julienp reopened this Jun 23, 2025
@julienp
julienp force-pushed the julienp/fail-test branch from ba2a173 to fd03861 Compare June 23, 2025 18:34
Comment thread .github/workflows/ci.yml
-test.parallel=8 -test.timeout=1h -test.v | sed 's/.*/[root] &/' &
docker run \
-e RUN_CONTAINER_TESTS=true \
-e IMAGE_VARIANT=pulumi-nonroot \

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.

Need to use matrix.variant for this

@julienp
julienp force-pushed the julienp/fail-test branch 3 times, most recently from ebd58d6 to 86d2890 Compare July 17, 2025 11:51
@julienp
julienp force-pushed the julienp/fail-test branch from 86d2890 to 42634f5 Compare July 17, 2025 13:22
@julienp
julienp merged commit 265a48d into main Jul 17, 2025
52 checks passed
@julienp
julienp deleted the julienp/fail-test branch July 17, 2025 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

impact/no-changelog-required This issue doesn't require a CHANGELOG update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants