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
github: Fix external workloads test file syntax #17019
Conversation
The external workloads jobs have passed. Removing the WIP commit and marking ready for the review. |
264fbaf
to
658e8e4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this. I forgot a git add
... 🤦🏻
Can you replace your changes with the following instead? These are the correct changes. Thanks :)
From 78427044d56f0c9bc5debabf8458729da431a43a Mon Sep 17 00:00:00 2001
From: Nicolas Busseneau <nicolas@isovalent.com>
Date: Fri, 30 Jul 2021 15:52:54 +0200
Subject: [PATCH] workflows: fix external workloads 1.10 YAML
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
---
.github/workflows/conformance-externalworkloads-v1.10.yml | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/.github/workflows/conformance-externalworkloads-v1.10.yml b/.github/workflows/conformance-externalworkloads-v1.10.yml
index fd55279a9..768a0bb99 100644
--- a/.github/workflows/conformance-externalworkloads-v1.10.yml
+++ b/.github/workflows/conformance-externalworkloads-v1.10.yml
@@ -72,8 +72,8 @@ jobs:
name: Deduce required tests from code changes
if: |
(github.event_name == 'issue_comment' && (
- (startsWith(github.event.comment.body, 'ci-external-workloads-v1.10') ||
- startsWith(github.event.comment.body, 'test-backport-1.10')) &&
+ startsWith(github.event.comment.body, 'ci-external-workloads-v1.10') ||
+ (startsWith(github.event.comment.body, 'test-backport-1.10'))
)) ||
(github.event_name == 'schedule' && github.repository == 'cilium/cilium') ||
github.event_name == 'pull_request'
@@ -112,8 +112,8 @@ jobs:
needs: check_changes
if: |
(github.event_name == 'issue_comment' && (
- startsWith(github.event.comment.body, 'ci-external-workloads') ||
- (startsWith(github.event.comment.body, 'test-me-please') && (needs.check_changes.outputs.tested == 'true'))
+ startsWith(github.event.comment.body, 'ci-external-workloads-v1.10') ||
+ (startsWith(github.event.comment.body, 'test-backport-1.10') && (needs.check_changes.outputs.tested == 'true'))
)) ||
(github.event_name == 'schedule' && github.repository == 'cilium/cilium') ||
github.event_name == 'pull_request'
--
2.32.0
@@ -76,7 +76,7 @@ jobs: | |||
if: | | |||
(github.event_name == 'issue_comment' && ( | |||
startsWith(github.event.comment.body, 'ci-external-workloads') || | |||
(startsWith(github.event.comment.body, 'test-me-please')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is correct and can be left as-is (consistent with other workflows).
startsWith(github.event.comment.body, 'ci-external-workloads-v1.10') || | ||
startsWith(github.event.comment.body, 'test-backport-1.10') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be:
startsWith(github.event.comment.body, 'ci-external-workloads-v1.10') || | |
startsWith(github.event.comment.body, 'test-backport-1.10') | |
startsWith(github.event.comment.body, 'ci-external-workloads-v1.10') || | |
(startsWith(github.event.comment.body, 'test-backport-1.10')) |
I am joining a patch with this change and another one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Does it make a difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's just to be consistent with the other workflows (just like for the other comment above). The failure was indeed related to the &&
, but GH suggestion shows the diff w.r.t. to the new code rather than the original one.
658e8e4
to
724240e
Compare
@nbusseneau Do you approve the latest change? |
@vadorovsky No, it is missing part of the patch, I think I was unclear sorry 😅 |
724240e
to
dbae23e
Compare
@nbusseneau PTAL |
dbae23e
to
28a5c80
Compare
The GH action runner reported: The workflow is not valid. .github/workflows/conformance-externalworkloads-v1.10.yml (Line: 73, Col: 9): Unexpected symbol: ')'. Located at position 185 within expression: (github.event_name == 'issue_comment' && ( (startsWith(github.event.comment.body, 'ci-external-workloads-v1.10') || startsWith(github.event.comment.body, 'test-backport-1.10')) && )) || (github.event_name == 'schedule' && github.repository == 'cilium/cilium') || github.event_name == 'pull_request' Fixes: 5726e9f ("workflows: add external workload conformance test") Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com> Signed-off-by: Martynas Pumputis <m@lambda.lt>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
See commit msgs.
Fixes: 5726e9f ("workflows: add external workload conformance test")