uPortal-Project / uPortal

Enterprise open source portal built by and for the higher education community.
https://www.apereo.org/projects/uportal
Apache License 2.0
272 stars 273 forks source link

Fix flaky tests: JSON comparison #2718

Closed KiruthikaJanakiraman closed 11 months ago

KiruthikaJanakiraman commented 11 months ago
Checklist
Description of change

This PR aims to fix the following 5 flaky test:

  1. org.apereo.portal.events.tincan.json.LrsDataModelSerializeTest.testLrsActorSerialize
  2. org.apereo.portal.events.tincan.json.LrsDataModelSerializeTest.testLrsObjectSerialize
  3. org.apereo.portal.events.AnalyticsIncorporationComponentEventSerializationTest.testMixinNoCopy
  4. org.apereo.portal.events.AnalyticsIncorporationComponentEventSerializationTest.testEventFiltering
  5. org.apereo.portal.events.tincan.json.LrsDataModelSerializeTest.testLrsStatementSerialize

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

Problem The test case org.apereo.portal.events.tincan.json.LrsDataModelSerializeTest.testLrsActorSerialize fails due to the below assertion as the order of the fields within the object lrsActor changes upon nondeterministic shuffling causing the order of JSON string result to change.

https://github.com/uPortal-Project/uPortal/blob/59f8895fe98782e1ff23dd14f9eb2076db09b04e/uPortal-webapp/src/test/java/org/apereo/portal/events/tincan/json/LrsDataModelSerializeTest.java#L52-L57

Solution This PR fixes the flakiness by using an ObjectMapper class to convert the JSON strings to JsonNode and then comparing the JsonNodes using JsonNode.equals method which performs a full(deep) comparison.

The other 4 test are also fixed similarly using this method.

Steps to reproduce The following command can be used to reproduce the assertion errors and verify the fix.

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 uPortal-webapp:test --tests org.apereo.portal.events.tincan.json.LrsDataModelSerializeTest.testLrsActorSerialize
./gradlew --info uPortal-webapp:test --tests org.apereo.portal.events.tincan.json.LrsDataModelSerializeTest.testLrsObjectSerialize
./gradlew --info uPortal-webapp:test --tests org.apereo.portal.events.tincan.json.LrsDataModelSerializeTest.testLrsStatementSerialize
./gradlew --info uPortal-webapp:test --tests org.apereo.portal.events.AnalyticsIncorporationComponentEventSerializationTest.testEventFiltering
./gradlew --info uPortal-webapp:test --tests org.apereo.portal.events.AnalyticsIncorporationComponentEventSerializationTest.testMixinNoCopy

Run NonDex:

./gradlew --info uPortal-webapp:nondexTest --tests=org.apereo.portal.events.tincan.json.LrsDataModelSerializeTest.testLrsActorSerialize --nondexRuns=50 -x autoLintGradle
./gradlew --info uPortal-webapp:nondexTest --tests=org.apereo.portal.events.tincan.json.LrsDataModelSerializeTest.testLrsObjectSerialize --nondexRuns=50 -x autoLintGradle
./gradlew --info uPortal-webapp:nondexTest --tests=org.apereo.portal.events.tincan.json.LrsDataModelSerializeTest.testLrsStatementSerialize --nondexRuns=50 -x autoLintGradle
./gradlew --info uPortal-webapp:nondexTest --tests=org.apereo.portal.events.AnalyticsIncorporationComponentEventSerializationTest.testEventFiltering --nondexRuns=50 -x autoLintGradle
./gradlew --info uPortal-webapp:nondexTest --tests=org.apereo.portal.events.AnalyticsIncorporationComponentEventSerializationTest.testMixinNoCopy --nondexRuns=50 -x autoLintGradle

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.

ChristianMurphy commented 11 months ago

Welcome @KiruthikaJanakiraman! 👋 Thanks for taking the time to contribute!

  1. When you have a chance, could you submit the Apereo ICLA? https://www.apereo.org/licensing/agreements/icla
  2. The tests are currently failing due to formatting, could you run ./gradlew goJF and commit the updated files?
  3. To confirm your goal, your goal is to reduce flakiness in CI/CD? (Very much appreciated) Or is there also a bug you are trying to reproduce?
KiruthikaJanakiraman commented 11 months ago

Welcome @KiruthikaJanakiraman! 👋 Thanks for taking the time to contribute!

  1. When you have a chance, could you submit the Apereo ICLA? https://www.apereo.org/licensing/agreements/icla
  2. The tests are currently failing due to formatting, could you run ./gradlew goJF and commit the updated files?
  3. To confirm your goal, your goal is to reduce flakiness in CI/CD? (Very much appreciated) Or is there also a bug you are trying to reproduce?

Hi Christian,

  1. I have submitted the ICLA.
  2. I ran the given command(./gradlew goJF) and committed the formatted files.
  3. My goal here is to remove the flakiness from test cases which I identified using the tool NonDex. NonDex explores different behaviors of under-determined APIs and reports test failures under different explored behaviors.