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

Update the code for logic of USE_TESTENV_PROPERTIES #4023

Merged
merged 2 commits into from
Oct 13, 2022

Conversation

julian55455
Copy link
Contributor

@julian55455 julian55455 commented Oct 9, 2022

Related #3337

cc @ShelleyLambert

@smlambert smlambert requested a review from sophia-guo October 9, 2022 20:20
Copy link
Contributor

@smlambert smlambert left a comment

Choose a reason for hiding this comment

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

Thanks @julian55455 - You can go one step further, since the if statements are now all the same, you will only need 1 of them. Leaving Line439 as it is, removing L443, L445, L449, L451.

Leaving a block that looks like this:

		if(!params.USE_TESTENV_PROPERTIES){
			String[] adoptSystemTest = getGitRepoBranch(params.ADOPTOPENJDK_SYSTEMTEST_OWNER_BRANCH, "adoptium:master", "aqa-systemtest")
			env.ADOPTOPENJDK_SYSTEMTEST_REPO = adoptSystemTest[0]
			env.ADOPTOPENJDK_SYSTEMTEST_BRANCH = adoptSystemTest[1]

			String[] openj9SystemTest = getGitRepoBranch(params.OPENJ9_SYSTEMTEST_OWNER_BRANCH, "eclipse:master", "openj9-systemtest")
			env.OPENJ9_SYSTEMTEST_REPO = openj9SystemTest[0]
			env.OPENJ9_SYSTEMTEST_BRANCH = openj9SystemTest[1]

			String[] stf = getGitRepoBranch(params.STF_OWNER_BRANCH, "adoptium:master", "STF")
			env.STF_REPO = stf[0]
			env.STF_BRANCH = stf[1]
		}

@smlambert
Copy link
Contributor

Grinder (test job using the PR branch):
xLinux, hs, jdk11: https://ci.adoptopenjdk.net/view/Test_grinder/job/Grinder/5767

@sophia-guo
Copy link
Contributor

There are two more places are using logic if(!params.USE_TESTENV_PROPERTIES) to doing setup. https://github.com/adoptium/aqa-tests/blob/master/buildenv/jenkins/JenkinsfileBase#L41 and https://github.com/adoptium/aqa-tests/blob/master/buildenv/jenkins/JenkinsfileBase#L427. #3337 mentioned to combine all those logic to one place https://github.com/adoptium/aqa-tests/blob/master/buildenv/jenkins/JenkinsfileBase#L41 .

It's fine to do it in two steps. First is this PR, @julian55455 I've updated your PR description to be related with #3337 instead of fix.

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