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

Adapted to most recent changes in churn - arguments support #5129

Merged
merged 12 commits into from
Mar 15, 2024

Conversation

judovana
Copy link
Contributor

@judovana judovana commented Mar 7, 2024

No description provided.

@judovana judovana marked this pull request as draft March 7, 2024 15:50
It canbe easily suplied by churn custom. Afaik it is better to remove it
then to disable it
@judovana
Copy link
Contributor Author

judovana commented Mar 9, 2024

Single gc 5h https://ci.adoptium.net/view/Test_grinder/job/Grinder/9064/tapResults/
all gc 60s total: https://ci.adoptium.net/view/Test_grinder/job/Grinder/9065/tapResults/

Will now try all platfroms, but looks ok to me: https://ci.adoptium.net/view/Test_grinder/job/Grinder/9066/

Hmm.. Hard to say if ZGC's failures on cigwin are as expected: [0.014s][error][gc] Failed to lookup symbol: VirtualAlloc2
And on s390x, both zgc and shenandoah reports that they do not exists, even if the -XX+enableThem is present.
Interesting

@judovana judovana marked this pull request as ready for review March 9, 2024 11:40
@judovana
Copy link
Contributor Author

@judovana
Copy link
Contributor Author

churn_5h_allGCs https://ci.adoptium.net/view/Test_grinder/job/Grinder/9077/
this long one, had not proopagated

rerun with archive artifacts: https://ci.adoptium.net/view/Test_grinder/job/Grinder/9079/

disabled.churn_custom "zgc shenandoah" 60 https://ci.adoptium.net/view/Test_grinder/job/Grinder/9076/

rerun with archive artifacts: https://ci.adoptium.net/view/Test_grinder/job/Grinder/9080/

disabled.churn_custom "zgc wrong" 60 https://ci.adoptium.net/view/Test_grinder/job/Grinder/9078/

Otherwise the jobs did what they should

@judovana
Copy link
Contributor Author

09:50:02  churn_5h_allGCs_0 Start Time: Mon Mar 11 08:50:02 2024 Epoch Time (ms): 1710147002275
09:50:02  variation: NoOptions
09:50:02  JVM_OPTIONS:  
09:50:02  { \
09:50:02  echo "";	echo "TEST SETUP:"; \
09:50:02  echo "Nothing to be done for setup."; \
09:50:02  mkdir -p "/home/jenkins/workspace/Grinder/aqa-tests/TKG/../TKG/output_17101470013550/churn_5h_allGCs_0"; \
09:50:02  cd "/home/jenkins/workspace/Grinder/aqa-tests/TKG/../TKG/output_17101470013550/churn_5h_allGCs_0"; \
09:50:02  echo "";	echo "TESTING:"; \
09:50:02  export JREJDK="jdk" ; \
09:50:02              export OTOOL_garbageCollector="ALL" ; \
09:50:02              export DURATION="18000" ; \
09:50:02              if [ "x" == "x" ] ; then  export CHURN_TAP=false  ; fi ; \
09:50:02              export TMPRESULTS="""/home/jenkins/workspace/Grinder/aqa-tests/TKG/../TKG/output_17101470013550/churn_5h_allGCs_0"/report""; \
09:50:02              cd MPRESULTS ; \
09:50:02                bash "/home/jenkins/workspace/Grinder/aqa-tests/TKG/../system/churn/churn/run.sh"; \
09:50:02              if [ $? -eq 0 ]; then echo "-----------------------------------"; echo "churn_5h_allGCs_0""_PASSED"; echo "-----------------------------------"; cd /home/jenkins/workspace/Grinder/aqa-tests/TKG/..;  else echo "-----------------------------------"; echo "churn_5h_allGCs_0""_FAILED"; echo "-----------------------------------"; fi; \
09:50:02  echo "";	echo "TEST TEARDOWN:"; \
09:50:02  echo "Nothing to be done for teardown."; \
09:50:02   } 2>&1 | tee -a "/home/jenkins/workspace/Grinder/aqa-tests/TKG/../TKG/output_17101470013550/TestTargetResult";
09:50:02  
09:50:02  TEST SETUP:
09:50:02  Nothing to be done for setup.
09:50:02  
09:50:02  TESTING:
09:50:02  /bin/sh: 10: [: x: unexpected operator
09:50:02  /bin/sh: 12: cd: can't cd to MPRESULTS

it is not healthy

@judovana
Copy link
Contributor Author

What kid of shell is running that?
I guess the fix for MPRESULTS will be ${TMPRESULTS} or $$TMPRESULTS ?

@judovana
Copy link
Contributor Author

@smlambert
Copy link
Contributor

Same issue you had with setting APPLICATION_OPTIONS, yes, needs $( )

Also, I should take a closer look at how this is run directly, as you should not need cd TMPRESULTS, since TKG sets up the temp location to run and gather results. Also, still seeing the rogue extra and differently formatted .tap file, which is unwanted, as its presence will cause confusion as the Jenkins scripts are expecting the TKG formated tap files, and automatically package them up to include for marketplace API.

@judovana
Copy link
Contributor Author

Same issue you had with setting APPLICATION_OPTIONS, yes, needs $( )

Also, I should take a closer look at how this is run directly, as you should not need cd TMPRESULTS, since TKG sets up

@jandrlik advised thi s to me, that it necessary to have the artifacts properly picked up.

the temp location to run and gather results. Also, still seeing the rogue extra and differently formatted .tap file, which is unwanted, as its presence will cause confusion as the Jenkins scripts are expecting the TKG formated tap files, and automatically package them up to include for marketplace API.

Exactly, the tap file should not be there. It is if-outed, but the

 /bin/sh: 10: [: x: unexpected operator

Is causing it reappear. In the currently pushed pushed version, the rogue tap file is missing. I just wanted to make it more explicit and the escaping had bitten me.

@judovana
Copy link
Contributor Author

@judovana
Copy link
Contributor Author

Being out of tmpresults is ok. All artifacts are packed.
The /bin/sh: 10: [: x: unexpected operator is still there. Any idea?

@judovana
Copy link
Contributor Author

@judovana
Copy link
Contributor Author

@judovana
Copy link
Contributor Author

churn_5h_allGCs https://ci.adoptium.net/view/Test_grinder/job/Grinder/9121
disabled.churn_custom https://ci.adoptium.net/view/Test_grinder/job/Grinder/9122

Now it should behave. Dash is no shell of mine :)

Copy link
Contributor

@karianna karianna left a comment

Choose a reason for hiding this comment

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

Number of typos

system/churn/README.md Outdated Show resolved Hide resolved
system/churn/README.md Outdated Show resolved Hide resolved
system/churn/README.md Outdated Show resolved Hide resolved
system/churn/playlist.xml Outdated Show resolved Hide resolved
system/churn/playlist.xml Outdated Show resolved Hide resolved
@judovana
Copy link
Contributor Author

churn_5h_allGCs https://ci.adoptium.net/view/Test_grinder/job/Grinder/9121 disabled.churn_custom https://ci.adoptium.net/view/Test_grinder/job/Grinder/9122

Now it should behave. Dash is no shell of mine :)

@smlambert The alien tapfile si finally gone from defautl job! \o/

@karianna ty! will fix asap

@judovana
Copy link
Contributor Author

@karianna thanx again for the additional spell check. I will be more careful next time. All applied.

export TMPRESULTS="$(Q)$(REPORTDIR)$(D)report$(Q)"; \
cd $TMPRESULTS ; \
bash "$(TEST_ROOT)$(D)system$(D)churn$(D)churn$(D)run.sh"; \
if [ "x${CHURN_TAP}" = "x" ] ; then export CHURN_TAP=true ; fi ; \
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the CHURN_TAP ?

Copy link
Contributor

Choose a reason for hiding this comment

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

A tap file that Jiri generates himself, which does not conform to the same information as the tap file that TKG generates, so I asked that it not be generated when running as part of AQAvit, as we use the TKG formated tap files for many purposes and having a non-conformant tap file on the system would be sub-optimal.

See comments in original PR starting here for background.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CHURN_TAP is control variable, which inside churn runner generate tap file - if empty or true. If it is set an non true, then tap file is not generated.

The tap file from churn is extremely useful so I let it on for the custom manual run. However it disturbed the automated tuns, so it is off for default auto run.

If you insists, I will turn it off also for manual run.

The condition is here, so the user from cmdline can control the behaviour.

Copy link
Contributor

Choose a reason for hiding this comment

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

Worth a comment in the code if this causes confusion

@judovana judovana requested a review from karianna March 14, 2024 16:53
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.

👍

@smlambert smlambert merged commit 802a3ed into adoptium:master Mar 15, 2024
2 checks passed
sophiaxu0424 pushed a commit to sophiaxu0424/aqa-tests that referenced this pull request Mar 19, 2024
…#5129)

* Adapted to most recent changes in churn - arguments support

* Improved application_options by $() subshell

* removed churn_1m_allGCs

It canbe easily suplied by churn custom. Afaik it is better to remove it
then to disable it

* Narrowed readme.md

* Using consistently $() instead of $ and ${}

* Returned {} instead of () for nkn-jenkins vars

* Replaced dash incompatible == by =

* removed forgotten _churn_1m_allGCs from readme

Co-authored-by: Martijn Verburg <[email protected]>

* fixed typos in system/churn/README.md

Co-authored-by: Martijn Verburg <[email protected]>

* One more remvoal of churn_1m_allGCs and from  system/churn/README.md

Co-authored-by: Martijn Verburg <[email protected]>

* remvoed redundant whitesapces in system/churn/playlist.xml

Co-authored-by: Martijn Verburg <[email protected]>

* more removed spaces in system/churn/playlist.xml

Co-authored-by: Martijn Verburg <[email protected]>

---------

Co-authored-by: Martijn Verburg <[email protected]>
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