Skip to content
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.

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

fix flaky test #5633

Merged
merged 2 commits into from
Jan 3, 2024
Merged

fix flaky test #5633

merged 2 commits into from
Jan 3, 2024

Conversation

Rette66
Copy link
Contributor

@Rette66 Rette66 commented Dec 5, 2023

Description

Fixed the flaky test when multiple attributes are specified it should examine all inside the AttributeSetTest class
Detected flaky test when setting an expectation as an object it should be serialized to json inside the DefaultMockServerTest class.
Detected flaky tests put /1, with missing item, should create item put /1, with existent item, should replace item get /1, with existent item, should return item get /, with one item, should return item get /, with multiple items, should return array inside the DefaultMockServerCrudTest class.

Root Cause

All tests mentioned above have been reported as flaky when run with the NonDex tool.

AttributeSetTest

when multiple attributes are specified it should examine all inside the AttributeSetTest class utilize Attribute and AttributeSet to perform test operation. However, groovy's AttributeSet does not guarantee the elements' order. Therefore, the indexOf() method will give different results based on the index of a3 at line 114.

AttributeSet selectorWithTwo = new AttributeSet(a2, a3);
then:
// Assert that the order is suitable for testing. The failing attribute should
// be in the *second* position to ensure we're examining all the values of the selector
assert new ArrayList<>(selectorWithTwo.attributes.values()).indexOf(a3) == 1;

DefaultMockServerTest

when setting an expectation as an object it should be serialized to json inside the DefaultMockServerTest class invokes the method clinent.newCall() which will return a response type result. Then convert this result to a string and compare it with the expected string. However, I guess the Response type will not guarantee elements order inside it. Therefore, converting Response to a string will generate a string that has a different order from the expected string.

Response response1 = client.newCall(request).execute()
then:
assert response1.code() == 200
assert response1.body().string() == "{\"id\":0,\"username\":\"root\",\"enabled\":true}"

DefaultMockServerCrudTest

Tests in this class have the same problem as the test in the DefaultMockServerTest class. Invoking to client.newCall() returns a Response type result that cannot guarantee a specific order for elements inside. Therefore, converting to a string will not generate the expected string.

def result = client.newCall(new Request.Builder().url(server.url("/")).build()).execute()
then:
assert result.code() == 200
assert result.body().string() ==
"[{\"id\":1,\"username\":\"user\",\"enabled\":true},{\"id\":2,\"username\":\"user-2\",\"enabled\":true}]"

To reproduce run:

mvn -pl junit/mockwebserver/ edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=<test_name>

Fix

AttributeSetTest

I fixed this test by creating a new LinkedHashSet to store Attribute. LinkedHashSet will guarantee the order of Attribute inside it.

DefaultMockServerTest & DefaultMockServerCrudTest

For tests in these two classes, a simple way to fix them is to create another Response object and assign expected data to the object. However, I cannot find any information about how to create a Response object and assign a value to it. Therefore, I can only discuss one possible solution here.

Key changed/added classes in this PR

  • io.fabric8.mockwebserver.crud.AttributeSetTest

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

Copy link

sonarqubecloud bot commented Jan 3, 2024

@manusa manusa added this to the 6.10.0 milestone Jan 3, 2024
@manusa manusa merged commit 74be92b into fabric8io:main Jan 3, 2024
17 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants