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

Added jtreg-buffer external suite #5167

Closed
wants to merge 11 commits into from

Conversation

judovana
Copy link
Contributor

@judovana judovana commented Mar 21, 2024

This PR is adding our buffer of not-yet-if-ever upstreamed jtreg tests, as another external testsuite.
It do pass locally, however in grinder it fails https://ci.adoptium.net/view/Test_grinder/job/Grinder/9229/console Error: cannot find the following tests: jtreg-buffer (note: group target such as sanity is not accepted inside testList) So i guess there is somwehre some another protection.

Except that issue, for which I do not have cure, few questions

  • external tests do not run with podman-docker docekr wrapper. The traces are quite cryptic, is this already tracked?
    • Is there generic/general opinion on moving to root-less container engine (thus podman)?
  • on grinder jobs, the external tests run as root? I had not found a trace of change of user in logs, so am wondering
    • local root-less run proiperly failed for me
  • is there any way how to copy any testsuite results from container to host?
  • the tap file is correctly on host, but the suite itself is generating jtrfiles and jtr.xml which I would llike to be able to archive on demand if there occure issues.

@judovana
Copy link
Contributor Author

judovana commented Apr 3, 2024

@sophia-guo maybe you can provide feedback please?

@karianna
Copy link
Contributor

karianna commented Apr 3, 2024

@judovana You should run a spell and grammar checker over the comments

@smlambert
Copy link
Contributor

smlambert commented Apr 3, 2024

PRs to add new tests or features should have an issue that they relate to, as that is what we typically do at the project to socialize new features.

I have created this issue, #5195, where it would say what the new tests do, why they should be added into a certain level, etc (noting we have the habit of adding new tests into the dev level to monitor how they run before 'graduating' tests into other levels).

@smlambert
Copy link
Contributor

https://ci.adoptium.net/view/Test_grinder/job/Grinder/9354 to fix the TARGET which needs to match the name in the playlist.xml file (not the name of the directory)

Also will want to address the criteria/questions listed in #5195 ahead of bringing in new tests. We will add this to the contributing new tests documentation going forward, so future contributors have awareness.

external/jtreg-buffer/README.md Outdated Show resolved Hide resolved
external/jtreg-buffer/README.md Outdated Show resolved Hide resolved
external/jtreg-buffer/README.md Outdated Show resolved Hide resolved
external/jtreg-buffer/README.md Outdated Show resolved Hide resolved
external/jtreg-buffer/README.md Outdated Show resolved Hide resolved
external/jtreg-buffer/test.properties Outdated Show resolved Hide resolved
external/jtreg-buffer/test.sh Outdated Show resolved Hide resolved
judovana and others added 7 commits April 4, 2024 08:25
Co-authored-by: Martijn Verburg <martijnverburg@gmail.com>
Co-authored-by: Martijn Verburg <martijnverburg@gmail.com>
Co-authored-by: Martijn Verburg <martijnverburg@gmail.com>
Co-authored-by: Martijn Verburg <martijnverburg@gmail.com>
Co-authored-by: Martijn Verburg <martijnverburg@gmail.com>
if returned, return in 3.9.6
Co-authored-by: Martijn Verburg <martijnverburg@gmail.com>
@judovana judovana requested a review from karianna April 4, 2024 06:29
@judovana
Copy link
Contributor Author

judovana commented Apr 4, 2024

@karianna thanx alot for eyball! All should be done as you requested. Need to try that maven in grinder now.
@smlambert Thanx! I will fix grinder job, and will answer the issue you created.

Can you please answer the remaining questions?

external tests do not run with podman-docker docekr wrapper. The traces are quite cryptic, is this already tracked?
    Is there generic/general opinion on moving to root-less container engine (thus podman)?

In meantime I had found that no, and created #5196 hth

on grinder jobs, the external tests run as root? I had not found a trace of change of user in logs, so am wondering

please?

    local root-less run properly failed for me
is there any way how to copy any testsuite results from container to host?

big please.

the tap file is correctly on host, but the suite itself is generating jtrfiles and jtr.xml which I would like to be able to archive on demand if there occurs issues.

@judovana
Copy link
Contributor Author

judovana commented Apr 4, 2024

https://ci.adoptium.net/view/Test_grinder/job/Grinder/9354 to fix the TARGET which needs to match the name in the playlist.xml file (not the name of the directory)

That do not work for me, not even locally. Locally the
BUILD_LIST=external/jtreg-buffer
TARGET=jtreg-buffer-test
is th eonly viable combination. Other ends with

 Error: cannot find the following tests: ..

Where ther above looks like what you are guggesting.. reruning anyway: https://ci.adoptium.net/view/Test_grinder/job/Grinder/9362/

Also will want to address the criteria/questions listed in #5195 ahead of bringing in new tests. We will add this to the contributing new tests documentation going forward, so future contributors have awareness.

On it. TYVM!

@judovana
Copy link
Contributor Author

judovana commented Apr 4, 2024

@judovana
Copy link
Contributor Author

judovana commented Apr 4, 2024

https://ci.adoptium.net/view/Test_grinder/job/Grinder/9363/

ok, so maven version is mandatory. The run now correctly passed. I'm not sure why... I left the target and bulild-list as in original one.. Well Maybe not... Probably harsh morning. Glad it is green.

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.

Technically OK with me, but Shelly to confirm if this is the right approach

@sophia-guo
Copy link
Contributor

on grinder jobs, the external tests run as root? I had not found a trace of change of user in logs

External tests are running inside container as root, which can be updated to create and use unprivileged user if root acess is not required by external tests.

@smlambert
Copy link
Contributor

Before we go too far with this PR, I think it would be good to think about these jtreg tests as possible ones to add into the openjdk group, given the intent was to eventually upstream them to that project (based on answers in #5195). I do not see a need to run them as part of the external group, they could go into dev.openjdk

@sophia-guo
Copy link
Contributor

sophia-guo commented Apr 4, 2024

is there any way how to copy any testsuite results from container to host?

To make testsuite results available it have to be copied to specific directory. In test.sh you can copy the test result to /testResults , update test.properties with test_results="testResults" and update command in playlist.xml specify --reportsrc . Similar to openliberty-mp-tck

This way all *.xml files are visible to junit plugin. Also check both ARCHIVE_TEST_RESULTS and KEEP_REPORTDIR will keep the results even when the tests pass.

@sophia-guo
Copy link
Contributor

I agree with Shelley. dev.openjdk might be a better category to go based on #5195 (comment)

@judovana
Copy link
Contributor Author

judovana commented Apr 5, 2024

is there any way how to copy any testsuite results from container to host?

To make testsuite results available it have to be copied to specific directory. In test.sh you can copy the test result to /testResults , update test.properties with test_results="testResults" and update command in playlist.xml specify --reportsrc . Similar to openliberty-mp-tck

This way all *.xml files are visible to junit plugin. Also check both ARCHIVE_TEST_RESULTS and KEEP_REPORTDIR will keep the results even when the tests pass.

Thanx a lot!

as test_results="testResults" is property, can it have any value, or is it jsut testResults waht is allowed? Also, what is it relative to?

@judovana
Copy link
Contributor Author

judovana commented Apr 5, 2024

Before we go too far with this PR, I think it would be good to think about these jtreg tests as possible ones to add into the openjdk group, given the intent was to eventually upstream them to that project (based on answers in #5195). I do not see a need to run them as part of the external group, they could go into dev.openjdk

Cool! Thanx!

I willexperiment a bit with test_results="testResults" then close this one and do a new into dev.openjdk

@judovana
Copy link
Contributor Author

judovana commented Apr 5, 2024

ok, https://ci.adoptium.net/view/Test_grinder/job/Grinder/9386/ seems to be correctly exporting results. Closing and moving to dev.openjdk

@judovana judovana closed this Apr 5, 2024
judovana added a commit to judovana/aqa-tests that referenced this pull request May 10, 2024
judovana added a commit to judovana/aqa-tests that referenced this pull request May 10, 2024
judovana added a commit to judovana/aqa-tests that referenced this pull request May 10, 2024
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