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

Add shaInfo target to collect all shas after the test run #651

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sophia-guo
Copy link
Contributor

@sophia-guo sophia-guo commented Dec 12, 2024

Part of adoptium/aqa-tests#5818

  1. add shaInfo target, which be called before the testSummary
  2. update getSha.txt to getShas.txt, which check all folders under TEST_ROOT, if there is .git then check the SHA and save the repo and sha
  3. remove the ant macro checkGitRepoSha

Signed-off-by: Sophia Guo <[email protected]>
@sophia-guo
Copy link
Contributor Author

@sophia-guo sophia-guo requested a review from smlambert December 12, 2024 16:14
@sophia-guo sophia-guo marked this pull request as ready for review December 12, 2024 16:20
@smlambert
Copy link
Contributor

This is a nice way to handle test material that is pulled in via git clone. I like it.

Separately we will need to think about how to deal with test material that is precompiled and run from jar files (such as dacapo and renaissance).

We need to check with others who use AQAvit before removing the ant macro, in case it is in use by anyone for internal vendor test material.

@sophia-guo
Copy link
Contributor Author

Have to work with adoptium/aqa-tests#5819

@sophia-guo sophia-guo requested a review from llxia December 13, 2024 15:25
settings.mk Outdated
@@ -364,7 +364,7 @@ $(SUBDIRS_TESTTARGET):

$(TESTTARGET): $(SUBDIRS_TESTTARGET)

_$(TESTTARGET): setup_$(TESTTARGET) rmResultFile $(TESTTARGET) resultsSummary teardown_$(TESTTARGET)
_$(TESTTARGET): setup_$(TESTTARGET) rmResultFile $(TESTTARGET) shaInfor resultsSummary teardown_$(TESTTARGET)
Copy link
Contributor

@llxia llxia Dec 13, 2024

Choose a reason for hiding this comment

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

  • Does it make sense to have shaInfor before running the test? In the event of a crash, we will still retain the SHA information.
  • Could we rename shaInfor to shaInfo?

Copy link
Contributor Author

@sophia-guo sophia-guo Dec 16, 2024

Choose a reason for hiding this comment

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

It can be done right after make compile. https://ci.adoptium.net/view/Test_grinder/job/Grinder/12117/ https://ci.adoptium.net/view/Test_grinder/job/Grinder/12118/

However it depends on what the crash is. If the crash exit the job instantly ( https://ci.adoptium.net/view/Test_grinder/job/Grinder/12124/console, this is not a test crash, still explain the scenario) , SHAs.txt is available but *.tap file unavailable, which won't help jenkins story unless we also archive it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to shaInfo

@llxia
Copy link
Contributor

llxia commented Dec 13, 2024

09:47:07  Collect all REPOs sha 
09:47:07  cd "/home/jenkins/workspace/Grinder/aqa-tests/TKG/../TKG/scripts"; \
09:47:07  bash "getSHAs.sh" --test_root_dir "/home/jenkins/workspace/Grinder/aqa-tests/TKG/.." --shas_file "/home/jenkins/workspace/Grinder/aqa-tests/TKG/../TKG/SHAs.txt" 
09:47:07  Check shas in /home/jenkins/workspace/Grinder/aqa-tests/TKG/.. and store the info in /home/jenkins/workspace/Grinder/aqa-tests/TKG/../TKG/SHAs.txt
09:47:07  touch /home/jenkins/workspace/Grinder/aqa-tests/TKG/../TKG/SHAs.txt
09:47:07  ================================================
09:47:07  timestamp: 20241212-144705
09:47:07  repo dir: /home/jenkins/workspace/Grinder/aqa-tests/TKG/../functional/security/Crypto/CryptoTest
09:47:07  git repo: 
09:47:07    Fetch URL: https://github.com/rh-openjdk/CryptoTest.git
09:47:07  sha:
09:47:07  f20591eb422ecfc2caef43fe2ce6100271e667b3
09:47:07  ================================================
09:47:07  timestamp: 20241212-144705
09:47:07  repo dir: /home/jenkins/workspace/Grinder/aqa-tests/TKG/../functional/security/ssl-tests/ssl-tests
09:47:07  git repo: 
09:47:07    Fetch URL: https://github.com/rh-openjdk/ssl-tests.git
09:47:07  sha:
09:47:07  79f368f9793ab015061b6f4c6010fb3c749491a5
09:47:07  ================================================
09:47:07  timestamp: 20241212-144705
09:47:07  repo dir: /home/jenkins/workspace/Grinder/aqa-tests/TKG/../functional/testHeadlessComponents/TestHeadlessComponents
09:47:07  git repo: 
09:47:07    Fetch URL: https://github.com/rh-openjdk/TestHeadlessComponents.git
09:47:07  sha:
09:47:07  633fa62bde9357a2a454534004bc8c5bba4034da
09:47:07  ================================================
09:47:07  timestamp: 20241212-144705
09:47:07  repo dir: /home/jenkins/workspace/Grinder/aqa-tests/TKG/..
09:47:07  git repo: 
09:47:07    Fetch URL: https://github.com/sophia-guo/openjdk-tests.git
09:47:07  sha:
09:47:07  5e1b49fd81e1a765a13ef2261201512db3d8a3c5
09:47:07  ================================================
09:47:07  timestamp: 20241212-144705
09:47:07  repo dir: /home/jenkins/workspace/Grinder/aqa-tests/TKG/../TKG
09:47:07  git repo: 
09:47:07    Fetch URL: https://github.com/sophia-guo/TKG.git
09:47:07  sha:
09:47:07  1d6dbc1f8ae0df928f511de95c57bac5a4f14bda

I do not see openj9 SHA in your Grinder.

@sophia-guo
Copy link
Contributor Author

sophia-guo commented Dec 16, 2024

SHA will be collected only if the REPO is used. That is say for openj9 if you run openj9 tests, the sha will be there.

My grinder is testing functional.dev and sanity.perf, which doesn't include openj9 tests. The reason for choosing those two categories is currently the SHAs for those two categories are missing. Similar to grinder if running openjdk tests against impl=hotspot there will be no openj9 repo SHA.

@llxia
Copy link
Contributor

llxia commented Dec 16, 2024

Openj9 sha was not captured even if we ran test in openj9 repo.

09:06:46  get functional test material...
09:06:46  git clone --depth 1 --reference-if-able /home/jenkins/openjdk_cache -b master https://github.com/eclipse/openj9.git
09:06:46  Cloning into 'openj9'...
09:07:35  Collect all REPOs sha 
09:07:35  cd "/home/jenkins/workspace/Grinder/aqa-tests/TKG/../TKG/scripts"; \
09:07:35  bash "getSHAs.sh" --test_root_dir "/home/jenkins/workspace/Grinder/aqa-tests/TKG/.." --shas_file "/home/jenkins/workspace/Grinder/aqa-tests/TKG/../TKG/SHAs.txt" 
09:07:35  Check shas in /home/jenkins/workspace/Grinder/aqa-tests/TKG/.. and store the info in /home/jenkins/workspace/Grinder/aqa-tests/TKG/../TKG/SHAs.txt
09:07:35  touch /home/jenkins/workspace/Grinder/aqa-tests/TKG/../TKG/SHAs.txt
09:07:35  ================================================
09:07:35  timestamp: 20241216-140734
09:07:35  repo dir: /home/jenkins/workspace/Grinder/aqa-tests/TKG/../TKG
09:07:35  git repo: 
09:07:35    Fetch URL: https://github.com/sophia-guo/TKG.git
09:07:35  sha:
09:07:35  1b194139d7c6f915905d3984a5dfc16aba9a0618
09:07:35  ================================================
09:07:35  timestamp: 20241216-140734
09:07:35  repo dir: /home/jenkins/workspace/Grinder/aqa-tests/TKG/..
09:07:35  git repo: 
09:07:35    Fetch URL: https://github.com/sophia-guo/openjdk-tests.git
09:07:35  sha:
09:07:35  c402bf749616655dc2d6d4ddcb51c1b7953a8ff6

https://ci.adoptium.net/view/Test_grinder/job/Grinder/12101/console

@sophia-guo
Copy link
Contributor Author

sophia-guo commented Dec 16, 2024

openj9 repo is removed once the material is moved https://github.com/adoptium/aqa-tests/blob/master/get.sh#L664, similar to vendors repo information. Hence no .git and no sha information.

Those information is still available by testenv.properties, which can be combined to SHAs.txt.

Or it's not necessary to remove those repo the workspace will be deleted for jenkins run. https://ci.adoptium.net/view/Test_grinder/job/Grinder/12113/ - dynamic compile
https://ci.adoptium.net/view/Test_grinder/job/Grinder/12114/ - non dynamic compile

@sophia-guo
Copy link
Contributor Author

I'm also thinking if the testenv.properties is necessary with *.tap get all SHAs information though it's a separate issue I will take a look later.

@smlambert
Copy link
Contributor

From a 'secure dev' perspective, if we are using anything from the a repo, even if its a makefile or a variable or two are set because of it (https://github.com/eclipse-openj9/openj9/blob/master/test/functional/testVars.mk), we should keep track the repo and the SHAs. What we ultimately want to be able to do is to fully reproduce a test run down to every detail.

@sophia-guo
Copy link
Contributor Author

See https://github.com/adoptium/aqa-tests/pull/5819/files, which won't remove openj9 or vendor's repo. So all SHAs can be collected by shaInfo targets

https://ci.adoptium.net/view/Test_grinder/job/Grinder/12113/ - dynamic compile
https://ci.adoptium.net/view/Test_grinder/job/Grinder/12114/ - non dynamic compile

Alternatively is for openj9 and vendors repos call getSHAs.sh in get.sh as before. I personally would prefer the first solution to keep the process simple and well maintained. Changes for both solutions will be in aqa-tests repo. This PR won't be affected.

@llxia
Copy link
Contributor

llxia commented Dec 17, 2024

See https://github.com/adoptium/aqa-tests/pull/5819/files, which won't remove openj9 or vendor's repo.

We work with multiple vendor repos (5 vendor repos and soon to be 6). Leaving unused material in the workspace can create confusion.

@sophia-guo
Copy link
Contributor Author

sophia-guo commented Dec 17, 2024

All vendors repos are cloned to its own destination https://github.com/adoptium/aqa-tests/blob/master/get.sh#L719 I'm not sure why it causes confusion.

I don't mind keep removing them if this is preferred.

PR adoptium/aqa-tests#5819 updated.

https://ci.adoptium.net/view/Test_grinder/job/Grinder/12131/
https://ci.adoptium.net/view/Test_grinder/job/Grinder/12132/

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.

4 participants