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

test: Fix missing artifacts for tests with parentheses #16540

Merged
merged 1 commit into from Jun 17, 2021

Conversation

pchaigno
Copy link
Member

@pchaigno pchaigno commented Jun 15, 2021

When tests with parentheses in their name fail, the artifacts are missing. This is happening because we run:

bash -c "zip -qr test_name.zip test_directory"

That therefore fails with:

/bin/bash: -c: line 0: syntax error near unexpected token `('

We need to add double quotes for this command to work properly with parentheses.

I tested this change with a previous version of this pull request where one test was made to fail on purpose.

Before:
image

After:
image

Fixes: #15128.
Fixes: #4176.

@pchaigno pchaigno added kind/bug/CI This is a bug in the testing code. area/CI Continuous Integration testing issue or flake release-note/ci This PR makes changes to the CI. labels Jun 15, 2021
@pchaigno pchaigno force-pushed the fix-missing-artifacts branch 3 times, most recently from 301b5c9 to bb8b34e Compare June 16, 2021 08:25
When tests with parentheses in their name fail, the artifacts are
missing. This is happening because we run:

    bash -c "zip -qr test_name.zip test_directory"

That therefore fails with:

    /bin/bash: -c: line 0: syntax error near unexpected token `('

We need to add double quotes for this command to work properly with
parentheses.

Fixes: b4bfb40 ("Test: Add test result in Jenkins Junit")
Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno pchaigno marked this pull request as ready for review June 16, 2021 10:31
@pchaigno pchaigno requested a review from a team as a code owner June 16, 2021 10:31
@pchaigno pchaigno requested a review from nebril June 16, 2021 10:31
@pchaigno pchaigno changed the title test: Fail the test on purpose test: Fix missing artifacts for tests with parentheses Jun 16, 2021
@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 17, 2021
@christarazi christarazi merged commit 41e830e into cilium:master Jun 17, 2021
@pchaigno pchaigno deleted the fix-missing-artifacts branch June 18, 2021 07:38
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.9 Jul 16, 2021
@pchaigno
Copy link
Member Author

Marking for backports as it can significantly help flake debugging.

@joestringer joestringer moved this from Needs backport from master to Backport done to v1.9 in 1.9.10 Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake kind/bug/CI This is a bug in the testing code. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
No open projects
1.9.10
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

CI: Missing artifact for K8sServicesTest Checks service across nodes Tests NodePort (kube-proxy)
5 participants