search

adoptium / aqa-tests

Home of test infrastructure for Adoptium builds
https://adoptium.net/aqavit
Apache License 2.0
127 stars 306 forks source link

testenvSettings.sh should only run once #5297

Open llxia opened 2 months ago

llxia commented 2 months ago

When USE_TESTENV_PROPERTIES=true and DYNAMIC_COMPILE=false, testenvSettings.sh gets triggered twice - once in compile.sh and once before make <test>. This does not cause any issues, but it is not necessary.

https://github.com/adoptium/aqa-tests/blob/83bec274a10eb52ac812af2acc50bb3ec4bc4673/compile.sh#L12 https://github.com/adoptium/aqa-tests/blob/83bec274a10eb52ac812af2acc50bb3ec4bc4673/buildenv/jenkins/JenkinsfileBase#L9

Console output:

00:01:13.427  + bash ./compile.sh
00:01:13.427  Set values based on ./testenv/testenv.properties:
00:01:13.427  =========
00:01:13.427  TKG_REPO=https://github.com/adoptium/TKG.git
00:01:13.427  TKG_BRANCH=master
...
00:02:11.016  + MAKE=make
00:02:11.016  + cd ./aqa-tests
00:02:11.016  + . ./scripts/testenv/testenvSettings.sh
00:02:11.016  ++ set +x
00:02:11.016  Set values based on ./testenv/testenv.properties:
00:02:11.016  =========
00:02:11.016  TKG_REPO=https://github.com/adoptium/TKG.git
00:02:11.016  TKG_BRANCH=master
...

We should add an if statement in https://github.com/adoptium/aqa-tests/blob/83bec274a10eb52ac812af2acc50bb3ec4bc4673/buildenv/jenkins/JenkinsfileBase#L9

run . ./scripts/testenv/testenvSettings.sh only if DYNAMIC_COMPILE == true

Sangyoon21 commented 2 months ago

I could work on it

Sangyoon21 commented 2 months ago

@Hana3706 She would push the pull request on this with me.

Sangyoon21 commented 2 months ago

@smlambert Sorry but could you reassign it to @Hana3706. She is working on it

smlambert commented 2 months ago

In order to assign to a contributor, they have to add a comment to the issue. @Hana3706 - please comment on this issue, to have it assigned to you. Thanks!

Hana3706 commented 2 months ago

I am so sorry for the confusion. I will work on this issue :)

smlambert commented 2 months ago

No problem Hana3706, it reminded us that we should update our Contributing.md document to mention that one must make a comment if they want to be assigned to an issue. :)

Hana3706 commented 1 month ago

I am just a bit confused about what further changes I should make to this issue. So I would be very thankful for a simple tip... Thanks :)

llxia commented 1 month ago

@Hana3706 Please see the comment: https://github.com/adoptium/aqa-tests/pull/5341#discussion_r1608685076

Hana3706 commented 1 month ago

I have updated the if statement however my new commit shows under issue #5341 and is pending for review (I apologize am super confused)

llxia commented 1 month ago

@Hana3706 https://github.com/adoptium/aqa-tests/issues/5297 is the issue and https://github.com/adoptium/aqa-tests/pull/5341 is the PR. It is correct for the code to show in https://github.com/adoptium/aqa-tests/pull/5341.