-
-
Notifications
You must be signed in to change notification settings - Fork 316
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
Add TAP pipeline #4285
Add TAP pipeline #4285
Conversation
|
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.
LGTM
|
||
usage () | ||
{ | ||
echo 'Usage : aqaTAp.sh --url <link to download TAP files> --dir <dir to store TAP files>' |
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.
echo 'Usage : aqaTAp.sh --url <link to download TAP files> --dir <dir to store TAP files>' | |
echo 'Usage : aqaTap.sh --url <link to download TAP files> --dir <dir to store TAP files>' |
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. I will update.
@@ -0,0 +1,183 @@ | |||
#!/usr/bin/env bash |
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.
Has this file been shellchecked?
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.
Is that a requirement? If so, do we have a doc for instruction?
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.
I ran the script at https://www.shellcheck.net/ and updated the script if the suggestion is applicable.
69625e6
to
2181647
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 Lan, I likely am missing something in the setup of the pipeline at https://ci.adoptopenjdk.net/view/Test_grinder/job/TAP_verification/ in order to test this PR. If you have a moment to take a quick look. I can not see your internal job.
def TARGETS = params.TARGETS.trim().split("\\s*,\\s*"); | ||
def LABEL = (params.LABEL) ?: "" | ||
def LABEL_ADDITION = (params.LABEL_ADDITION) ?: "" |
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.
Are TARGETS and LABEL_ADDITION used/needed?
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.
- https://ci.adoptopenjdk.net/view/Test_grinder/job/TAP_verification/11/console does not work because this PR has not been merged.
aqaTap.sh
does not exist in master. I can do a demo this afternoon. TARGETS
is intended for future use. I would like users to provide what TARGET they want to check, not just be limited to the predefined AQA 9 targets.- yes,
LABEL_ADDITION
is not needed. It should be removed.
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.
I have pointed the main config to your branch (where aqaTap.sh exists), but realize I need to run with CUSTOMIZED_SDK_URL_CREDENTIAL_ID.
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.
ah, your fork is openjdk-tests, not aqa-tests, I see my testing dilemna !
resolves: adoptium#4277 Signed-off-by: Lan Xia <[email protected]>
resolves: #4277
Signed-off-by: Lan Xia [email protected]