Skip to main content

Please review suggested fix for CR 6662752 "Provide ability to mutate Test"

1 reply [Last post]
bavik
Offline
Joined: 2004-09-22
Points: 0

Index: D:/Projects/Javatest/src/com/sun/javatest/TestResult.java
===================================================================
--- D:/Projects/Javatest/src/com/sun/javatest/TestResult.java (revision 620)
+++ D:/Projects/Javatest/src/com/sun/javatest/TestResult.java (working copy)
@@ -997,12 +997,6 @@
* @param value The new value of the specified property.
*/
public synchronized void putProperty(String name, String value) {
- // check mutability
- if (!isMutable()) {
- throw new IllegalStateException(
- "Cannot put property, the TestResult is no longer mutable!");
- }
-
props = PropertyArray.put(props, name, value);
notifyUpdatedProperty(name, value);
}
@@ -1498,7 +1492,7 @@
File resultsDir = resultsFile.getParentFile();
resultsDir.mkdirs(); // ensure directory created for .jtr file

- File tempFile = createTempFile(workDir, backupPolicy);
+ File tempFile = createTempFile();
try {
writeResults(tempFile, backupPolicy);
}
@@ -1507,13 +1501,39 @@
tempFile.delete();
}
}
+
+ /**
+ * Saves changes in test result made after test execution to the same test result file.
+ */
+ public synchronized void mutateSavedResults(BackupPolicy backupPolicy) throws IOException {
+ if (isMutable()) {
+ throw new IllegalStateException("This TestResult is still mutable - set the status!");
+ }
+
+ if (resultsFile == null || !resultsFile.exists()) {
+ throw new IllegalStateException("This TestResult has not been saved yet.");
+ }
+
+ if (props == null) {
+ props = emptyStringArray;
+ }

+ File tempFile = createTempFile();
+ try {
+ writeResults(tempFile, backupPolicy);
+ } finally {
+ if (tempFile.exists()) {
+ tempFile.delete();
+ }
+ }
+ }
+
/**
* Create a temporary file to which the results can be written, before being renamed
* to its real name.
*/
// don't use File.createTempFile because of issues with the internal locking there
- private File createTempFile(WorkDirectory workDir, BackupPolicy backupPolicy)
+ private File createTempFile()
throws IOException
{
final int MAX_TRIES = 100; // absurdly big limit, but a limit nonetheless

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
bkurotsu
Offline
Joined: 2004-12-13
Points: 0

I think this is too fundamental of a change to make to the harness at this point. It has been an invariant that the test result cannot change after the final status has been set.

Although it may seem that the fix works, we don't know what side effects it may have. This includes effects due to caching and the fact that it will change the timing of changes which could theoretically result in exposing race conditions. If this part of the harness breaks, it will result in a harness which may not be usable.

Allowing this change also eliminates any possibility in the future of aggressive caching, since the result may change at any time and be rewritten.