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 AdoptOpenJDK #2024

Closed
aahlenst opened this issue Oct 20, 2020 · 12 comments
Closed

Add smoke tests for AdoptOpenJDK #2024

aahlenst opened this issue Oct 20, 2020 · 12 comments

Comments

@aahlenst
Copy link
Contributor

In https://github.com/adoptopenjdk/openjdk-build, we configure a lot of things without actually testing/ensuring that they have the desired effect. Examples:

  • Vendor names in version string
  • Special build configuration (JFR, Shenandoah, ...)
  • Custom CA certificates
  • Metadata

To make things worse, some of these things are version/platform dependant (for example, Shenandoah).

This needs to be covered by tests to reduce breakages and to ensure the long-term maintainability of the project.

@aahlenst
Copy link
Contributor Author

First PR (check TLS/SSL stack) has landed: #2049.

@karianna karianna added this to the November 2020 milestone Nov 12, 2020
@smlambert
Copy link
Contributor

I believe many of these can be added into functional/packaging or functional/build directories.

Created JDK_VENDOR (adoptium/TKG#115) and JDK_VERSION enhancment (adoptium/TKG#116) issues so that we can come back and update playlists appropriately (does not block us from adding tests that temporarily do their own checking of vendor and update versions in the interim).

@aahlenst
Copy link
Contributor Author

Something to cover with tests: adoptium/temurin-build#2229

@aahlenst
Copy link
Contributor Author

@aahlenst
Copy link
Contributor Author

aahlenst commented Nov 21, 2020

@smlambert
Copy link
Contributor

After virtual deskside review of #2140 with @adamfarley and also considering #2067, felt it would be useful to create a table of the known specific tests, and noting how one may verify it, in order to see the common patterns we can implement to cover all the known cases and better prepare for new unknown cases.

Note: This table is incomplete and will undergo several revisions (need to verify which vendors each test case is applicable to), and add new known/needed test cases to the list, even if they are not implemented or are in a PR at present.

Test for How to verify Impls Vendors Version(s) Platform(s)
testIfCudaIsEnabled find file + find string in file openj9 openj9,ibm all win/linux
FreetypeAbsenceTest.testTemplateExample find file all all? all linux
freetypeOnlyBundledOnWindowsAndMacOS find file all all? all on Win/MacOS, not on Linux/AIX
testShenandoahAvailable cmdLineTest hotspot adopt,upstream/RH 15+,11.0.9+ all aarch64, x86
testZGCAvailable cmdLineTest hotspot adopt 15+ aarch64 linux, x64 win/macos/linux
testJFRAvailable cmdLineTest hotspot adopt 11+, 8u262 ---
javaVersionPrintsVendor cmdLineTest all --- --- ---
vmPropertiesPointToVendor cmdLineTest all --- --- ---
testDefaultTls ability to connect to servers all all? all all
testTls13 ability to connect to servers all all? 11+ all

Given this, we see we need generic methods, findFile, findStringInFile, checkCommandLine via ProcessBuilder or cmdLineTester, connectToServer via HttpsURLConnection. All utility methods to find version info, or build info can be moved to a common area for use by all tests (we have many implementations of similar type utility methods scattered around, and an effort should be made to consolidate some of them).

@adamfarley
Copy link
Contributor

adamfarley commented Jan 7, 2021

@aahlenst - Based on our respective PRs (#2140 and #2067), Shelley's comments above, and your comment in #2140, I'd say we've got several items to complete here. Since much of the work is already done, here's my draft of a todo list.

Please review and let me know your thoughts.

  1. To complete WIP: Add vendor-specific tests for jdk builds #2140 (including the addition of the "comprehensive" change, along with the proposed shared functionality) and merge, forming the basis for the underlying framework for the build tests. (Adam)
    Reasoning: The vendor-agnostic framework, related documentation, and test template allows other vendors like dragonwell to use it. Also, the shared functionality in "BuildIs" would lend well to Shelley's plan for more shared functionality later.
  2. To adapt Add first batch of smoke tests for AdoptOpenJDK #2067's test suite to fit into the vendor-agnostic framework, using shared functionality where possible. (Andreas, with Adam willing to help).
    Reasoning: As above. Also, using shared functionality for vendor-determination makes it trivial to find later if we remove it as part of the TestNg paramaterization stuff (where a test is only compiled and run if it is tagged with a specific vendor. Discussed with Shelley, details tbd.)
  3. To decide on a final home for the shared functionality (build version string parsing, findFile, findStringInFile, etc) and to move the shared code there (perhaps TKG?). Also includes the work adapting the tests if needed, and also any documentation. (All)
    Reasoning: We already have a copy of stuff like the "build version string parsing" elsewhere, and we should make this as widely available as possible to avoid folks having to duplicate work implimenting it, and also copying fixes if they change the version string upstream. Also mentioned in Shelley's comment.
  4. To adapt the framework to pass the vendor name in via TestNG parametric functionality. (owner tbd)
    Reasoning: This, to prevent tests even being considered for compilation if it's not being run on a build by a suitable vendor. (To be discussed in more detail once the vendor parameter reaches us from upstream).
  5. To create an issue including the identification of other duplicates of our shared code. To delete them or merge them, and to ensure tests are not broken by this. (owner tbd).

@aahlenst
Copy link
Contributor Author

aahlenst commented Jan 8, 2021

Both PRs are stuck because we haven't yet decided how to organize the tests and how to select the tests. Shelley said a while ago that she made some experiments but I do not know what the outcome was.

From my POV, there are multiple ways to tackle this. What we have seen so far:

  • The current capabilities to filter tests in the XML are insufficient because we cannot express ranges down to the x.y.z level or say "Apply to 11, 15 and newer" or do that on a per-test basis. There are also cases where the test needs to be altered depending on the version (different VM args, for example).
  • Auto detection of VM vendor based on VM properties is risky because VM properties are sometimes wrong.
  • Differences between vendors are large. Take TLS 1.3 support in JDK 8: Azul introduced it a long time before everyone else, around 8u222 or so. Then came Oracle and half a year later OpenJDK. Red Hat does still not enable JRF for their builds, Oracle ignores Shenandoah and Red Hat even includes it in 8.

Because of that, I think the approach we both took (determining in the test whether something is supported) is wrong and will lead to unmaintainable and very complicated tests. But the question is: What's the alternative?

First aspect, versions a.k.a time: Usually, test material is saved alongside the code to be tested. So if you change the code, you run the tests that you find in an adjacent directory and they all must pass. There's no "Uhm, take the test suite from a year ago and it should be green for the current code" or "What about using today's test suite for a one year old release. Sure they'll pass." But by using version numbers as some sort of feature flags ("must be present in 8u272 or newer but absent before"), we try exactly that. This is madness and we should stop. Even though our test material is separate, who in their right mind would run today's test suite against a build of 8u192? So we go back to "That's a test for 11, that's for 8" and call it a day. After a test has been added, it is expected that all newer builds pass. If there are material changes required while one version is in rampdown (e.g. 8u282) and the other one is in development (e.g. 8u292), we're screwed. But that's never going to happen 🤞And if it does, there's still ProblemList.txt or a human that can ignore the failure.

Vendors: We have seen that VM props can be unreliable. Therefore, I'd argue that the vendor selection needs to be an argument set by a human. We usually know what we're building anyway. This cuts down on potential problems. Now that we know what the vendor is, how do we know what tests to run? Having a single test for everyone and letting it figure out what values to expect puts a big burden on the author (you need to check dozens of builds even if you have zero interest in a subset of products) and the tests become very complex. Just look at the number of conditions required for Shenandoah. This again is madness and we need to stop. Alternatives:

  1. Supplying a file per vendor that specifies which tests to run.
  2. Having dedicated folders per vendors, each with a full copy of "their" tests.

Option 1 is sufficient for tests like TLS 1.3 or Shenandoah because those boil down to "Run test: yes/no". However, it does not work for tests of VM properties or the list of jmods which vary per vendor/platform. So we're back at having long if-elseif-else blocks or would require something like Cucumber specs.

Option 2 would do away with the if-elseif-else blocks (simple tests, yay) but comes at the expense of having duplicated or triplicated tests that vary slightly. Is it so bad? Upside: Maintenance of vendor specific folders could be outsourced to the respective teams.

Last aspect, how do we combine vendor and version? Having a folder per version in a vendor's directory (like dragonwell/11) looks attractive at first (we know exactly which tests apply) but isn't: A test introduced with 11 would have to be copied into every future version's folder. Using the XML that we have now would solve this problem by being able to express ranges, but is only slightly better because a single XML applies to all tests in a suite. But as we have seen, each test can apply to a different set of versions. This would result in a large number of suites, most containing only a single test. So we need some mechanism that works on a per-file basis like the annotations in jtreg tests.

To sum up, I'd end up with folders per vendors and simple version range expressions per test. But maybe I missed some aspects or it doesn't fit with what we have at all. What I've completely ignored are platform differences, but maybe that's something we have to ultimately solve within each test or could be done in a similar way as the version ranges, for example through some sort of annotation per file.

As soon as we have decided what we want, we can make the necessary changes to TKG/... Factoring out common functionality (as well as using a better assertion library) can come afterwards and does not look like such a big deal to me.

@smlambert
Copy link
Contributor

This code has been delivered into openjdk-build repo via adoptium/temurin-build#2397

@smlambert
Copy link
Contributor

re: #2024 (comment) JDK_VENDOR support is now in TKG and soon we will look at adding minor version support also, in the meanwhile the code that handles minor version support stays in the test code, but when it sits at that level, it has to be repeated in each group of test code, and results in a lot of needless duplication.

@andrew-m-leonard
Copy link
Contributor

java.vendor.version change merged: adoptium/temurin-build#2544

@smlambert
Copy link
Contributor

I will close this as completed, minor fixes for now incorporated smoke tests are being done via fresh issues in the openjdk-build repo.

@karianna karianna added this to the April 2021 milestone Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants