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

Remove extra space in Rerun build with failed targets link #4074

Merged
merged 10 commits into from
Oct 31, 2022

Conversation

Nuel-Eneji
Copy link
Contributor

Input trim() to check trailing whitespace

I added the trim() to remove whitespace before and after the string

this draft PR attempts to fix issue #4063

singed-off-by: Nuel Eneji enejiemmanueladie@gmail.com

@Nuel-Eneji Nuel-Eneji marked this pull request as ready for review October 23, 2022 23:03
@Nuel-Eneji Nuel-Eneji marked this pull request as draft October 23, 2022 23:05
@Nuel-Eneji
Copy link
Contributor Author

Hello, @smlambert I hope everything is good with you.
I just made a draft pull request as directed and used the trim() function where it was thought to be suitable.
I would like to take part in the PR review process.
Thanks

@Nuel-Eneji
Copy link
Contributor Author

Hello @smlambert I'm yet to get a response from you.
I'll like to know the progress

@llxia
Copy link
Contributor

llxia commented Oct 26, 2022

@Nuel-Eneji could you please remove the changes under .github/ISSUE_TEMPLATE? I believe they are not related to the fix of #4063

@Nuel-Eneji
Copy link
Contributor Author

Hello @llxia sorry for the inconvinience, I've reverted the changes as made.
Thanks for the review

@@ -1073,7 +1073,7 @@ def addFailedTestsGrinderLink(paths=""){
def failedTestList = ""
List<String> buildPaths = paths.split(',')
for (def buildPath: buildPaths) {
def tapFiles = findFiles(glob: "${buildPath}**/*.tap")
def tapFiles = findFiles(glob: "${buildPath.trim()}**/*.tap")
Copy link
Contributor

@llxia llxia Oct 26, 2022

Choose a reason for hiding this comment

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

This will not fix the problem. I think trim() should be added at L1084

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @llxia I'll get on with that now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @llxia Thanks for the review. Please i need further guidiance on this
Thanks

@llxia llxia changed the title .trim() Remove extra space in Rerun build with failed targets link Oct 26, 2022
@Nuel-Eneji
Copy link
Contributor Author

Hello @smlambert @llxia I moved the trim() function to L1084 as suggested. Please can you help me explain why you suggested the function be placed on that line as I'm not to sure of what type of data passes through the functions on that line
Thanks

@llxia
Copy link
Contributor

llxia commented Oct 31, 2022

https://ci.adoptopenjdk.net/view/Test_grinder/job/Grinder/6071/
Rerun in Grinder with failed test targets (see below). Extra space in TARGET is removed as expected.

https://ci.adoptopenjdk.net/job/Grinder/parambuild/?SDK_RESOURCE=customized&TARGET=testList+TESTLIST=cmdLineTester_jvmtitests_0,cmdLineTester_jvmtitests_1,cmdLineTester_jvmtitests_2,cmdLineTester_jvmtitests_4,cmdLineTester_jvmtitests_5,cmdLineTester_jvmtitests_6,cmdLineTester_jvmtitests_7,cmdLineTester_jvmtitests_8&TEST_FLAG=&UPSTREAM_TEST_JOB_NAME=&DOCKER_REQUIRED=false&ACTIVE_NODE_TIMEOUT=&VENDOR_TEST_DIRS=&EXTRA_DOCKER_ARGS=-v+%24%7BTEST_JDK_HOME%7D%3A%2Fopt%2Fjava%2Fopenjdk&TKG_OWNER_BRANCH=AdoptOpenJDK%3Amaster&OPENJ9_SYSTEMTEST_OWNER_BRANCH=eclipse-openj9%3Amaster&PLATFORM=x86-64_linux&GENERATE_JOBS=false&KEEP_REPORTDIR=true&PERSONAL_BUILD=true&ADOPTOPENJDK_REPO=https%3A%2F%2Fgithub.com%2FNuel-Eneji%2Faqa-tests.git&RERUN_ITERATIONS=2&LABEL=&EXTRA_OPTIONS=&CUSTOMIZED_SDK_URL=https%3A%2F%2Fopenj9-artifactory.osuosl.org%2Fartifactory%2Fci-openj9%2FBuild_JDK11_x86-64_linux_Nightly%2F403%2FOpenJ9-JDK11-x86-64_linux-20221021-000008.tar.gz&BUILD_IDENTIFIER=&ADOPTOPENJDK_BRANCH=remove-unwanted-space&LIGHT_WEIGHT_CHECKOUT=false&NON_AQA_TEST_REPOS=&ARTIFACTORY_SERVER=&KEEP_WORKSPACE=false&USER_CREDENTIALS_ID=&JDK_VERSION=11&ITERATIONS=1&VENDOR_TEST_REPOS=&JDK_REPO=&OPENJ9_BRANCH=master&OPENJ9_SHA=&JCK_GIT_REPO=&VENDOR_TEST_BRANCHES=&OPENJ9_REPO=https%3A%2F%2Fgithub.com%2Feclipse%2Fopenj9.git&UPSTREAM_JOB_NAME=&CLOUD_PROVIDER=&PLATFORM_AND_MACHINE=&CUSTOM_TARGET=&VENDOR_TEST_SHAS=&JDK_BRANCH=&TEST_IMAGES_REQUIRED=false&LABEL_ADDITION=&ARTIFACTORY_REPO=&ARTIFACTORY_ROOT_DIR=&UPSTREAM_TEST_JOB_NUMBER=&DOCKERIMAGE_TAG=&JDK_IMPL=openj9&SSH_AGENT_CREDENTIAL=&AUTO_DETECT=true&DYNAMIC_COMPILE=true&ADOPTOPENJDK_SYSTEMTEST_OWNER_BRANCH=adoptium%3Amaster&CUSTOMIZED_SDK_URL_CREDENTIAL_ID=&OPENJDK_SHA=&NUM_MACHINES=&BUILD_LIST=functional&USE_TESTENV_PROPERTIES=false&UPSTREAM_JOB_NUMBER=&STF_OWNER_BRANCH=adoptium%3Amaster&TIME_LIMIT=10&JVM_OPTIONS=&PARALLEL=None

Please can you help me explain why you suggested the function be placed on that line

The issue happens in testSet.getTestResult(i).getDescription(), not buildPath

Copy link
Contributor

@llxia llxia left a comment

Choose a reason for hiding this comment

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

LGTM

@smlambert smlambert marked this pull request as ready for review October 31, 2022 19:13
@smlambert smlambert merged commit bc24835 into adoptium:master Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants