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 Smoke tests for build & packaging #2397

Merged
merged 9 commits into from
Feb 5, 2021
Merged

Conversation

smlambert
Copy link
Contributor

Signed-off-by: Shelley Lambert <slambert@gmail.com>
Signed-off-by: Shelley Lambert <slambert@gmail.com>
Signed-off-by: Shelley Lambert <slambert@gmail.com>
Signed-off-by: Shelley Lambert <slambert@gmail.com>
Signed-off-by: Shelley Lambert <slambert@gmail.com>
Signed-off-by: Shelley Lambert <slambert@gmail.com>
@smlambert smlambert added the enhancement Issues that enhance the code or documentation of the repo in any way label Jan 21, 2021
@smlambert
Copy link
Contributor Author

smlambert commented Jan 21, 2021

Still to do:

  • determine if any tests from FeatureTests.java are applicable to openj9 impl, assuming no, update test targets in playlist accordingly
  • all functional tests can/should share utility methods for checking version, etc.
  • if we want VendorPropertiesTest for each vendor built at the project, use parameterization
  • both impls are failing javaVendorVersion test, is it considered non-blocking? related: java.vendor.version only displays "AdoptOpenJDK" #1387
  • change PR to include co-contributors

@M-Davies
Copy link

Would these tests be used alongside our existing suite?

@smlambert
Copy link
Contributor Author

The intent of these tests is to check that no one has made breaking changes (to what gets packaged for release). This smoke set will grow to have several more checks, and could also include installer testing.

They could get used alongside (or triggered at the same time) depending on when the existing set runs. How/where is the existing suite triggered? Part of PR testing? part of a pipeline run? or both? (yes, I can look to answer this myself ;) )

@M-Davies
Copy link

M-Davies commented Jan 21, 2021

They're executed as part of the Compile pr tester on all build PR's (see https://github.com/AdoptOpenJDK/openjdk-build/tree/master/pipelines/build/prTester#Compile). You can also execute it locally:

cd pipelines
 ./gradlew --info test

@karianna karianna added this to the January 2021 milestone Jan 22, 2021
@smlambert
Copy link
Contributor Author

These smoke tests are meant to verify the build artifact that gets created (actually call java executable after its unpacked onto a machine), so they are not suitable to add to the Compile PR tester workflow.

I will look at the various points in our pipeline where they would be better applied, and note that they can even still exist as a standalone job rather than a step or a stage in the build pipeline.

(refering to the diagram at AdoptOpenJDK/TSC#158 (comment) they could be the first subtest run after sign step and before any other tests run, and definitely before publish).

@karianna
Copy link
Contributor

karianna commented Feb 3, 2021

@smlambert - Did you want to continue with this PR or are you happy to park it given our refactoring efforts?

@aahlenst
Copy link
Contributor

aahlenst commented Feb 3, 2021

@karianna Getting this PR in should be the single top-priority of this project.

@karianna
Copy link
Contributor

karianna commented Feb 3, 2021

@karianna Getting this PR in should be the single top-priority of this project.

In that case @smlambert - if you're happy with it can you take it out of draft and we can review and try to land.

@smlambert
Copy link
Contributor Author

re: #2397 (comment)
There are a couple of other things that should happen, but some can come in as subsequent PRs

adoptium/TKG#142 has landed, I can update the SmokeTestsForBuildAndPackaging test job to not use my TKG branch now.

The SmokeTestsForBuildAndPackaging job is running as a standalone, and should also get tied into the overall build pipeline (so that it can block further testing and publishing of build artifacts).

Signed-off-by: Shelley Lambert <slambert@gmail.com>
Signed-off-by: Shelley Lambert <slambert@gmail.com>
@smlambert smlambert changed the title WIP: Add Smoke tests for build & packaging Add Smoke tests for build & packaging Feb 4, 2021
@smlambert smlambert marked this pull request as ready for review February 4, 2021 03:09
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.

LGTM - I have a preference for one assert per test but it's not as hill I'm going to die on :-)

Copy link

@M-Davies M-Davies left a comment

Choose a reason for hiding this comment

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

Does any documentation in this repo need updating to inform/educate users of these changes?

Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

On the basis that this is just the test cases being added which are called without requiring changes to the main build pipelines, I see no reason not to merge this [*]

[*] - Other than the fact things are a little unstable today as a result of the fallout from merging #2132

@smlambert
Copy link
Contributor Author

smlambert commented Feb 4, 2021

Yes, still to do:

  • add Andreas and Adam as co-authors

There will be PRs to follow this one:

  • Trigger the smoke test job from the openjdk_build_pipeline.groovy with something like:
  /* 
    Run tests from the build repository that verify certain features are present and enabled,
    and system properties are correct.  These tests block further testing and publishing of 
    build artifacts.
    */
    def runBuildSmokeTests() {
        def jobName = "SmokeTestsForBuildandPackaging"
...
  • Figure out where to put documentation (guidance for other build and packaging tests to follow, for example, guidance to someone adding new tests to verify contents of RELEASE file - related: Correct "HEAP_SIZE" release file value for x86 OpenJ9 builds #2413)
  • testZGCAvailable is not failing when in error (see example) - I will see if I can figure that out today and either push to this PR, or follow with a separate one. EDIT: it is behaving as expected, command line options used to simply verify the presence of the feature, acceptable to not include -XX:+UnlockExperimentalVMOptions on the command line to enable (especially if it shifts out of experimental in the future).

Co-authored-by: Andreas Ahlenstorf <andreas@ahlenstorf.ch>
Co-authored-by: Adam Farley <adfarley@redhat.com>
Co-authored-by: Shelley Lambert <slambert@gmail.com>
@smlambert
Copy link
Contributor Author

@M-Davies - per your question regarding doc, I will plan to add doc in the follow-up PR where this gets tied into the build pipeline itself (so the doc can describe that these tests will block other testing and publishing of artifacts).

Currently anyone can trigger the SmokeTestsForBuildandPackaging job, against any build produced at the project (just as you would any Grinder).

Given there are 2 approvals on this, I will merge it and begin on the subsequent PR so that we automatically rather than manually launch the test job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues that enhance the code or documentation of the repo in any way
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants