Fixed a few non deterministic tests in the repository

[yugvajani]

Fix a few non deterministic tests in the repository

This pull request resolves several non-deterministic tests in the repository, addressing some of the tests mentioned in the issue openhab-core issue #4474.

There were a number of flaky tests in the repository which were detected using the NonDex tool. More details about the steps to reproduce it and the details for the tools are mentioned in openhab-core issue #4474.

Description

This PR fixes the following tests mentioned below and the steps to reproduce the issue is already mentioned in openhab-core issue #4474:

For the tests:

• org.openhab.core.thing.link.events.LinkEventFactoryTest.testCreateItemChannelLinkRemovedEvent
• org.openhab.core.thing.link.events.LinkEventFactoryTest.testCreateItemChannelLinkAddedEvent
• org.openhab.core.io.rest.Stream2JSONInputStreamTest.shouldStreamCollectionStreamToJSON
• org.openhab.core.io.rest.Stream2JSONInputStreamTest.shouldStreamSingleObjectToJSON
• org.openhab.core.config.discovery.inbox.events.InboxEventFactoryTest.inboxEventFactoryCreatesInboxAddedEventCorrectly
• org.openhab.core.items.events.ItemEventFactoryTest.testCreateCommandEventOnOffType
• org.openhab.core.items.events.ItemEventFactoryTest.testCreateStateEventOnOffType

The reason for flakiness is that since JSON-like structure is being compared using assertEquals, order dependency could cause the test to fail. JSON properties may not always maintain the same order, depending on the implementation of the object creation or serialization.

A JSON parser was utilized to process the objects before comparison. This ensures the content is evaluated irrespective of property order, resolving the order dependency issue.

For the test:

• org.openhab.core.cache.ExpiringCacheMapTest.testValues

The test org.openhab.core.cache.ExpiringCacheMapTest.testValues failed due to non-deterministic ordering in the subject.values() function, causing assertEquals to fail.

The values were converted to a LinkedHashSet before comparison. This guarantees a consistent, deterministic order.

For the tests:

• org.openhab.core.config.core.internal.validation.ConfigDescriptionValidatorTest.assertValidationThrowsExceptionContainingMessagesForAllMinMaxConfigParameters
• org.openhab.core.config.core.internal.validation.ConfigDescriptionValidatorTest.assertValidationThrowsExceptionContainingMessagesForMultipleInvalidTypedConfigParameters
• org.openhab.core.config.core.internal.validation.ConfigDescriptionValidatorTest.assertValidationThrowsExceptionContainingMultipleVariousViolations
• org.openhab.core.config.core.internal.validation.ConfigDescriptionValidatorTest.assertValidationThrowsExceptionContainingMessagesForAllRequiredConfigParameters

The flakiness in the tests are caused by non-deterministic ordering of the ConfigValidationMessage objects in the exception. The params map's entries are iterated in a non-deterministic order when processed by the configDescriptionValidator.validate method. This can result in the order of ConfigValidationMessage objects in the exception being inconsistent across different test runs.

To fix the test, I made use of the containsInAnyOrder matcher which ensures the comparison focuses on the content of the messages without depending on their order.

For the tests:

• org.openhab.core.model.yaml.internal.YamlModelRepositoryImplTest.testUpdateElementInModel
• org.openhab.core.model.yaml.internal.YamlModelRepositoryImplTest.testAddElementToModel

The reason for flakiness is that YAML is a human-readable data serialization format, but it does not guarantee a deterministic order for keys in mappings. If the YamlModelRepositoryImpl modifies the content or serializes the added element in a different key order than expected, the string comparison (is(expectedFileContent)) will fail, even if the data structures are logically equivalent.

The fix involves making use of YAML parser to load the file contents as structured objects ensuring order independence.

[wborn]

If you like fixing flaky tests, we have a few more. 😉

See: https://github.com/openhab/openhab-core/issues?q=is%3Aopen+is%3Aissue+label%3Atest

[yugvajani]

If you like fixing flaky tests, we have a few more. 😉

See: https://github.com/openhab/openhab-core/issues?q=is%3Aopen+is%3Aissue+label%3Atest

Certainly, I'll take a look at these. In the meantime, could you review my changes and let me know if they're ready to merge?

[yugvajani]

@wborn could you provide the steps to reproduce the failed tests?

[yugvajani]

@wborn Do I need an account to access Jules logs? The Jenkins link provided in the issue appears to be invalid, and I am unable to reproduce the flaky tests locally.

[wborn]

The Jenkins link provided in the issue appears to be invalid, and I am unable to reproduce the flaky tests locally.

The builds get removed after a while to clean up space so that's why the links stop working.

Most of the unstable tests are probably due to timing issues.

It's sometimes easier to reproduce when you put a lot of CPU load on your computer while running the tests.

During the CI builds there is also a lot of CPU load because it runs as a parallel Maven build (-T 1.5C option).

The timing is also influenced by OS specific JVM implementation details.

[yugvajani]

It's sometimes easier to reproduce when you put a lot of CPU load on your computer while running the tests.

Did you use any Jenkins plugin for detecting flaky tests, such as the Flaky Test Handler Plugin, or could you recommend one which I could use?

[holgerfriedrich]

I would recommend not to add any plugin to our supply chain which has such a small developer community.
Same applies to maven dependencies added to our build/test steps.

As Wouter mentioned before, you can just
run the tests with more threads in parallel and you will see timing issues.

[wborn]

Did you use any Jenkins plugin for detecting flaky tests, such as the Flaky Test Handler Plugin

I just create an issue everytime a test fails that normally succeeds. That makes it easier to figure out when another build also fails if it is due to a flaky test. Then when I'm in a "flaky test fixing mood" I try to fix them. 😉