On Wed, 10 May 2023 14:52:58 GMT, Mahendra Chhipa <mchh...@openjdk.org> wrote:
>> Test is updated to create the binary files during test execution.
>
> Mahendra Chhipa has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Implemented the review comments.
Apologies for the delay, with the RDP1. fast approaching, it has been difficult
getting back to this due to other priorities.
Please see the comments below as I feel we can do more to make the tests more
streamline/maintainable.
Another datapoint to consider is that this might be a good time to convert the
test from _testng_ to _junit_ as that is the preferred direction for our future
tests.
test/jdk/javax/xml/jaxp/datatype/8033980/SerializationTest.java line 114:
> 112: // generatePseudoCodeForDurationSerBytes(baos2);
> 113: // generatePseudoCodeForDurationSerBytesAsBase64(base64dur);
> 114: }
I really do not like this being hidden in the setup method
I still would look to have a method that actually creates the classes. One
example of this:
`open/test/jdk/java/io/Serializable/failureAtomicity/FailureAtomicity.java` and
the `createSrc` method
The intent is to make this easier to update/enhance going forward. I would
probably have. separate utility class/template which can be run to create the
saved serialization classes.
SerializationTest would have an introductory comment outlining how to add
additional serialized at a via the utility class.
test/jdk/javax/xml/jaxp/datatype/8033980/SerializationTest.java line 125:
> 123: @DataProvider(name = "GregorianCalendarData")
> 124: public Object [][] gregorianCalendarDataBytes() {
> 125: return new Object [][] {{System.getProperty("java.version"),
> gregorianCalendarAndDurationSerData[0], EXPECTED_CAL},
We really only need to call `System.getProperty("java.version")` once in the
program and then save off its return value
test/jdk/javax/xml/jaxp/datatype/8033980/SerializationTest.java line 188:
> 186: final ObjectInputStream ois = new ObjectInputStream(bais);
> 187: final XMLGregorianCalendar xgc = (XMLGregorianCalendar)
> ois.readObject();
> 188: Assert.assertEquals(xgc.toString(), gregorianDate);
if you:
`import static org.testng.Assert.*;`. then you can simply call `assertEquals`
-------------
PR Review: https://git.openjdk.org/jdk/pull/13537#pullrequestreview-1451413985
PR Review Comment: https://git.openjdk.org/jdk/pull/13537#discussion_r1210567715
PR Review Comment: https://git.openjdk.org/jdk/pull/13537#discussion_r1210569626
PR Review Comment: https://git.openjdk.org/jdk/pull/13537#discussion_r1210580188
