Skip to content

fix flaky tests in the file EntityToSchemaMapperTest.java #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix flaky tests in the file EntityToSchemaMapperTest.java #1

wants to merge 1 commit into from

Conversation

ThugJudy
Copy link
Owner

@ThugJudy ThugJudy commented Nov 7, 2023 •
edited
Loading

This PR aims to fix the following 5 flaky test cases:

io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testLink
io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testMapAsDataItem
io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testMapAsExposed
io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testMapAsLinks
io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testUriTemplate

I found and confirmed the flaky behavior using an open-source research tool NonDex, which shuffles implementations of nondeterminism operations.

Problem:
The writeValueAsString() function is nondeterministic and when it tries to convert a YAML object to a string, it does not account for the ordering of elements of the YAML object.

spring-data-rest-utils/src/main/java/io/github/vlsergey/springdatarestutils/SchemaUtils.java

Line 33 in 8cd30df

return Yaml.pretty().writeValueAsString(obj);

Hence, all the tests mentioned above when they try to call the SchemaUtils.writeValueAsString() function, are prone to flakiness.

Solution:

Since we do not have access to the internal library, I have added a check, that converts the YAML string to YAML objects and then compares the objects.

Steps to reproduce

Integrate NonDex:

Add the following snippet to the top of the build.gradle in $PROJ_DIR:

plugins {
    id 'edu.illinois.nondex' version '2.1.1-1'
}

Append to build.gradle in $PROJ_DIR:

apply plugin: 'edu.illinois.nondex'

Execute Test with Gradle:

./gradlew --info test --tests io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testLink
./gradlew --info test --tests io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testMapAsDataItem
./gradlew --info test --tests io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testMapAsExposed
./gradlew --info test --tests io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testMapAsLinks
./gradlew --info test --tests io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testUriTemplate

Run NonDex:

./gradlew --info nondexTest --tests=io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testLink 
./gradlew --info nondexTest --tests=io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testMapAsDataItem 
./gradlew --info nondexTest --tests=io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testMapAsExposed 
./gradlew --info nondexTest --tests=io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testMapAsLinks 
./gradlew --info nondexTest --tests=io.github.vlsergey.springdatarestutils.EntityToSchemaMapperTest.testUriTemplate

Test Environment:

Java version "1.8.0_381"
macOS Venture Version 13.4.1 (22F82)

Please let me know if you have any concerns or questions.

@zzjas
Copy link

zzjas commented Nov 14, 2023

Probably you want to remove the text The following command can reproduce the assertion errors and verify the fix. since there's no command immediately below it. Also please add links to the tests you fixed and put them within backticks.

zzjas
zzjas reviewed Nov 14, 2023
View reviewed changes
src/test/java/io/github/vlsergey/springdatarestutils/EntityToSchemaMapperTest.java Outdated
@@ -30,10 +33,11 @@ class EntityToSchemaMapperTest {

@Test
void testLink() throws Exception {
Yaml yaml = new Yaml(new Constructor(Map.class));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can all the Yaml yaml = new Yaml(new Constructor(Map.class)); be extracted out and shared?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

src/test/java/io/github/vlsergey/springdatarestutils/EntityToSchemaMapperTest.java Outdated
final Schema<?> schema = mapper.mapEntity(Link.class, ClassMappingMode.DATA_ITEM, RequestType.RESPONSE);
String json = SchemaUtils.writeValueAsString(false, schema);
Map<String, Object> json = yaml.load(SchemaUtils.writeValueAsString(false, schema));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird to name it json when it's yaml. Does SchemaUtils.writeValueAsString(false, schema) return a YAML string?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@zzjas zzjas mentioned this pull request Nov 14, 2023
Open
@ThugJudy
Copy link
Owner Author

Probably you want to remove the text The following command can reproduce the assertion errors and verify the fix. since there's no command immediately below it. Also please add links to the tests you fixed and put them within backticks.

I have added the hyperlink for all the failing test

zzjas
zzjas reviewed Nov 15, 2023
View reviewed changes
src/test/java/io/github/vlsergey/springdatarestutils/EntityToSchemaMapperTest.java Outdated

class EntityToSchemaMapperTest {

private static final ScanResult emptyScanResult = new ScanResult(emptyMap(), emptySet(), emptySet(), emptySet());

private final TaskProperties taskProperties = new TaskProperties().setAddXLinkedEntity(true);

private final Yaml yaml = new Yaml(new Constructor(Map.class));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra indentation?

src/test/java/io/github/vlsergey/springdatarestutils/EntityToSchemaMapperTest.java Outdated
}

@Test
void testMapAsDataItem() throws Exception {
final Schema<?> schema = mapper.mapEntity(TestEntity.class, ClassMappingMode.DATA_ITEM, RequestType.RESPONSE);
String json = SchemaUtils.writeValueAsString(false, schema);
Map<String, Object> json = yaml.load(SchemaUtils.writeValueAsString(false, schema));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you change above to be yamlMap, you want to change all occurrence to keep consistent.

src/test/java/io/github/vlsergey/springdatarestutils/EntityToSchemaMapperTest.java Outdated
}

@Test
void testUriTemplate() throws Exception {
void testUriTemplate() throws Exception {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra indentation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zzjas zzjas zzjas left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ThugJudy @zzjas