Gaffer icon indicating copy to clipboard operation
Gaffer copied to clipboard

Fixing three flaky tests in AuthorisationsTest and ViewTest

Open Sitara-a opened this issue 1 year ago • 6 comments

Two flaky tests were fixed in this pull request-

  1. uk.gov.gchq.gaffer.commonutil.elementvisibilityutil.AuthorisationsTest.testToString
  2. uk.gov.gchq.gaffer.data.elementdefinition.view.ViewTest.shouldCreateAnIdenticalObjectWhenCloned
  3. uk.gov.gchq.gaffer.data.elementdefinition.view.ViewUtilTest.shouldCreateAnIdenticalObjectWhenCloned

Flakiness in the tests -

To check for flakiness in the tests, a tool called nondex was used.

Steps to reproduce flakiness using nondex -

Test 1:

mvn install -pl core/common-util -am -DskipTests
mvn -pl core/common-util test -Dtest=uk.gov.gchq.gaffer.commonutil.elementvisibilityutil.AuthorisationsTest
mvn -pl core/common-util edu.illinois:nondex-maven-plugin:2.1.7:nondex -Dtest=uk.gov.gchq.gaffer.commonutil.elementvisibilityutil.AuthorisationsTest

The third command shows us the flakiness in the test. There are test failures due to differences in the order of elements image

Test 2:

mvn install -pl core/data -am -DskipTests
mvn -pl core/data test -Dtest=uk.gov.gchq.gaffer.data.elementdefinition.view.ViewTest
mvn -pl core/data edu.illinois:nondex-maven-plugin:2.1.7:nondex -Dtest=uk.gov.gchq.gaffer.data.elementdefinition.view.ViewTest

The third command shows us the flakiness in the test. There are test failures due to differences in order of elements image

Test 3:

mvn install -pl core/data -am -DskipTests
mvn -pl core/data test -Dtest=uk.gov.gchq.gaffer.data.elementdefinition.view.ViewUtilTest.shouldCreateAnIdenticalObjectWhenCloned
mvn -pl core/data edu.illinois:nondex-maven-plugin:2.1.7:nondex -Dtest=uk.gov.gchq.gaffer.data.elementdefinition.view.ViewUtilTest.shouldCreateAnIdenticalObjectWhenCloned

Source of Flakiness in tests and fixes:

  1. AuthorisationsTest.testToString The Authorisation object created in the test case appends three strings to the auths member of the class which is a HashSet. The toString() method of the class then iterates over this HashSet and builds a String after concatenating the members of the set. Since a Java HashSet does not store order, the test case fails if the elements of the HashSet are iterated over in any order other than the order in which they were appended during object instantiation. To fix this, the test was updated to check if all strings in the auths member of the object are a part of the String returned by toString() instead of focusing on the order.
image
  1. ViewTest.shouldCreateAnIdenticalObjectWhenCloned In this test case, the source of flakiness is the following lines - final byte[] viewJson = view.toCompactJson(); final byte[] cloneJson = clone.toCompactJson();

assertThat(cloneJson).containsExactly(viewJson);

The method toCompactJson() internally uses JSONSerialiser.serialise() to convert the object into a byte stream. This does not preserve the order of attributes. Therefore, the containsExactly() method fails when the order of attributes differs in the two objects. To fix this, an ObjectMapper object was created and two JsonNode objects were created after calling the readTree() method on the two byte arrays. The equality check was then applied on these two JsonNode objects.

  1. ViewUtilTest.shouldCreateAnIdenticalObjectWhenCloned Similar to test 2, the source of flakiness is from the toCompactJson() method which does not preserve the order of attributes.

Please let me know if you have any questions or need any additional justification/changes from my side.

Sitara-a avatar Oct 28 '24 18:10 Sitara-a

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Oct 28 '24 18:10 CLAassistant

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 67.94%. Comparing base (73b7544) to head (0bcd9cf).

Additional details and impacted files
@@            Coverage Diff             @@
##             develop    #3327   +/-   ##
==========================================
  Coverage      67.94%   67.94%           
  Complexity      2596     2596           
==========================================
  Files            955      955           
  Lines          30530    30530           
  Branches        3369     3369           
==========================================
  Hits           20744    20744           
  Misses          8306     8306           
  Partials        1480     1480           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 29 '24 16:10 codecov[bot]

Looks good to me.

omkrpt avatar Nov 07 '24 09:11 omkrpt

Have fixed a couple more tests (https://github.com/gchq/Gaffer/pull/3155, https://github.com/gchq/Gaffer/pull/3156) do let me know if they make sense @Sitara-a

omkrpt avatar Nov 07 '24 10:11 omkrpt

Have fixed a couple more tests (#3155, #3156) do let me know if they make sense @Sitara-a

Yup! The fixes look good to me.

Sitara-a avatar Nov 18 '24 01:11 Sitara-a