RFR: 7903519 : jtreg/jtharness is missing features for basic crash testing [v9]

[Dmitry Bessonov]

On Thu, 4 Jan 2024 10:51:55 GMT, Dmitry Bessonov <dbessono at openjdk.org> wrote:

>> andrlos has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   random folder usage in the unit test
>
> For the purposes of additionally processing and changing test execution statuses you can create a subclass of `Script` (or subclass of its child class your are using) and override `::initTestDescription` method where you can use a custom `TestResult` with an overridden `::setStatus` method doing the needed transformations.

> @dbessono first of all - thank you for your kind input. Second - in my experience with both jtharness and its integration into jtreg I don't think that the case you are trying to imply is as simple as one might think it is. There are very cryptic ways of how the TestResult is being propagated down and back up and initial testing of your idea gave me NPE as the TestResult gets lost if I try to meddle with those two methods alone and I believe it has to do something with the test internal urls and the shrinklist... I will see how deep this rabbit hole gets me. Third - as we would want to have the option to modify the input built directly into the either jtreg or jtharness, is there any real issue of integrating this option directly to jtharness with this very simple piece of code? as the only advantage of using your proposed solution would be to keep jtharness code from changing and forcing us to do the changes in the jtreg code instead. Again thank you very much for your input.

Thanks for suggesting the customization.

Ah, can see that the `testResult` field is private. The original idea of `Script` base class is to allow users to subclass it and override the needed parts. So to avoid introduction of new concepts, interfaces etc. and to make customization simpler we can allow subclasses of `Script` to partially override the stages test execution flow (instead of forcing to completely reimplement method `::run`) by extracting phases of `::run` that could be overridden if needed, say smth like:


diff --git a/src/com/sun/javatest/Script.java b/src/com/sun/javatest/Script.java
--- a/src/com/sun/javatest/Script.java	(revision 22e872054eec91c94da79f8358cb010a33788755)
+++ b/src/com/sun/javatest/Script.java	(date 1704717221851)
@@ -465,7 +465,7 @@
 
         testResult.setEnvironment(env);
         testResult.putProperty("totalTime", Long.toString(System.currentTimeMillis() - startMs));
-        testResult.setStatus(execStatus);
+        fillTestExecutionStatus(testResult, execStatus);
 
         try {
             if (execStatus.getType() != Status.PASSED || jtrIfPassed) {
@@ -477,6 +477,13 @@
         }
     }
 
+    /**
+     * TODO
+     */
+    protected void fillTestExecutionStatus(TestResult testResultToFill, Status execStatus) {
+        testResultToFill.setStatus(execStatus);
+    }
+
     /**
      * The primary method to be provided by Scripts. It is responsible for compiling
      * and executing the test appropriately.  Normally, a script should call `init' and



Note that `protected TestDescription td;` field is accessible to the overriders.

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

PR Comment: https://git.openjdk.org/jtharness/pull/57#issuecomment-1880927227