[rfc][icedtea-web] add DeploymentPropertiesModifierTests

[Jie Kang]

----- Original Message -----
> Hello,
> 
> This patch adds four tests for DeploymentPropertiesModifierTests.

Hello,

Nice work!

Nits below:


Could you also rename the class DeploymentPropetiesModifier to DeploymentPropertiesModifier in this patch?


Can you use a temporary file for the tests instead of the user's Deployment Configuration file? This way you can pre-write data to the file, possibly simplify the tests and not mess around with user's file if the class doesn't restore properly.


+        for (String line : properties.split("\n")) {
+            if (line.contains(DeploymentConfiguration.KEY_SECURITY_LEVEL)) {
+                assertEquals(setProperty, line);
+            }
+        }

For something like this, a shorter way of writing this would be something along the lines of:
  assertTrue(properties.contains("\n" + var + "\n");


+        if (savedLine == null) {
+            assertFalse(foundLine);
+        } else {
+            assertTrue(foundLine);
+        }

Rather than checking the single line, it'd be faster/easier to just check the whole file's contents to match for proper restoration. Or, if you use temporary configuration file setup before the tests, you wouldn't need the null check stuff as well. In general, I feel like there is some over-complication in testSetAndRestoreProperties. 

I think there could be a few more tests like:

testSetProperties :: only sets
testRestoreProperties :: only restores and rather than using DPM, manually clobbering the deployment configuration file with stuff :: this test should not need to rely on DPM's set capability working to run


Everything else look fine to me.


Regards,

> 
> Thank you,
> Lukasz Dracz

-- 

Jie Kang

OpenJDK Team - Software Engineering Intern