RFR: 8254129: IR Test Framework to support regex-based matching on the IR in JTreg compiler tests [v2]

[Igor Ignatyev]

On Mon, 19 Apr 2021 15:34:27 GMT, Igor Ignatyev <iignatyev at openjdk.org> wrote:

>>> * although having javadoc for testlibraries is highly desirable, I don't think we should check in the generated HTML files
>>> * the same goes for `README.html` generated from `README.md`
>> 
>> Okay, I will remove them. Does it make sense to still have the HTML files somewhere in the web, for example, on my cr.openjdk? 
>> 
>>> * this library is hotspot-centric, I highly doubt that it will be used by any tests outside of the hotspot test base, hence the more appropriate location for it would be inside `test/hotspot/jtreg/testlibrary`.
>>> * I assume `test/lib/jdk/test/lib/hotspot/ir_framework/tests/` are the tests for the framework, in that case they should be in `test/lib-test`, if we stick to `test/lib` as the location for the library, or in `test/hotspot/jtreg/testlibrary_tests`, if we move it to `test/hotspot`
>> 
>> That makes sense to move everything to `test/hotspot/jtreg/testlibrary`. Right, the `test/lib/jdk/test/lib/hotspot/ir_framework/tests/` are only tests for the framework itself and should not be run as part of tier testing each time (does not make much sense) but only when the framework is actually modified. Is this still the case when putting them in `test/hotspot/jtreg/testlibrary_tests` (i.e. not executed unless run manually)?
>> 
>> I will do this things in a separate commit and adjust the README.md file accordingly (has links to the Javadoc files).
>
>> > * although having javadoc for testlibraries is highly desirable, I don't think we should check in the generated HTML files
>> > * the same goes for `README.html` generated from `README.md`
>> 
>> Okay, I will remove them. Does it make sense to still have the HTML files somewhere in the web, for example, on my cr.openjdk?
>> 
>> > * this library is hotspot-centric, I highly doubt that it will be used by any tests outside of the hotspot test base, hence the more appropriate location for it would be inside `test/hotspot/jtreg/testlibrary`.
>> > * I assume `test/lib/jdk/test/lib/hotspot/ir_framework/tests/` are the tests for the framework, in that case they should be in `test/lib-test`, if we stick to `test/lib` as the location for the library, or in `test/hotspot/jtreg/testlibrary_tests`, if we move it to `test/hotspot`
>> 
>> That makes sense to move everything to `test/hotspot/jtreg/testlibrary`. Right, the `test/lib/jdk/test/lib/hotspot/ir_framework/tests/` are only tests for the framework itself and should not be run as part of tier testing each time (does not make much sense) but only when the framework is actually modified. Is this still the case when putting them in `test/hotspot/jtreg/testlibrary_tests` (i.e. not executed unless run manually)?
> 
> `test/hotspot/jtreg/testlibrary_tests` are seldomly run as part of `:hotspot_misc` test group, yet I don't think it's an issue. the benefit of running testlibrary tests is to gain confidence that the tests which use these libraries are reliable. I'd also disagree that `ir_framework/tests` should be run *only* when the framework is actually, they should also be run when its dependencies are changed, and the framework highly depends on hotspot, so to be sure that the framework didn't get broken after changes in c2, whitebox, etc, you do need run these tests more often.
> 
> Thanks,
> -- Igor

> There is no decision, yet, (and open for discussion) about the location and package name for the framework and the framework internal tests. After discussing it offline with @iignatev, we think there are basically two good options:
> 
> 1. Leave the framework in `/test/lib` and put the framework internal tests into `/test/lib-test`:
>    
>    * Pros: Only need to specify `@library /test/lib` in a JTreg test; the framework internal tests are run in tier1 before any other tests are run which depend on the framework ensuring correctness.
>    * Cons: Clarity: The framework is intended to be used for HotSpot tests only (thus not exactly the right location in `/test/lib`); the framework internal tests might be run too often as part of tier1 (trade-off ensuring correctness vs. execution time).
> 2. Move the framework to `/test/hotspot/jtreg/testlibrary`, put the framework internal tests into `/test/hotspot/jtreg/testlibrary_tests`, and use package name `hotspot.test.lib.ir_framework`:
>    
>    * Pros: Clarity: The framework is only used for HotSpot tests (mainly compiler tests but could also be used for other tests).
>    * Cons: A JTreg test needs to additionally specify `/testlibrary/ir_framework` like `@library /testlibrary/ir_framework /test/lib`; the framework internal tests are run more seldomly as part of `:hotspot_misc` (trade-off, see cons of first option).
> 

there is also 3rd alternative, move the framework to `/test/hotspot/jtreg/compiler/` and use `compiler.ir_framework` or `compiler.lib.ir_framework` as the package name. that will make it even clearer that this is a compiler-specific library, and its users are going to be in `/test/hotspot/jtreg/compiler/`. I understand that there can be some runtime (or other) tests that might benefit from this library, but in the end, they won't be clear runtime tests, so `t/h/j/compiler` might be a better location for them anyways. the `@library` tag will still have to contain two paths, but these will be usual paths as in many compiler tests : `@library / /test/lib`.
the tests can be placed in `/test/hotspot/jtreg/testlibrary_tests` as in the 2nd option, with the same trade-off.

-- Igor

-------------

PR: https://git.openjdk.java.net/jdk/pull/3508