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

Fixing flaky test in LogbackLoggerOverridesTest #499

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mihirgune
Copy link

Description

This change is meant to fix a flaky test - testSave_writtenInLogbackXMLFormat() in LogbackLoggerOverridesTest.java

Analysis

This test presents flakiness because the ordering of loggerLevels in LogbackLoggerOverrides.java is non-deterministic.

This can be reproduced by running the following command using the nondex plugin.

mvn -pl components/nexus-base edu.illinois:nondex-maven-plugin:2.1.7:nondex -Dtest="LogbackLoggerOverridesTest"

throwing the error

[ERROR] Failures:
[ERROR]   LogbackLoggerOverridesTest.testSave_writtenInLogbackXMLFormat:64
Expected: is "<?xml version='1.0' encoding='UTF-8'?>\r\n\r\n<!--\r\nDO NOT EDIT - Automatically generated; User-customized logging levels\r\n-->\r\n\r\n<included>\r\n  <logger name='bar' level='INFO'/>\r\n  <property name='root.level' value='WARN'/>\r\n  <logger name='foo' level='ERROR'/>\r\n</included>\r\n"
     but: was "<?xml version='1.0' encoding='UTF-8'?>\r\n\r\n<!--\r\nDO NOT EDIT - Automatically generated; User-customized logging levels\r\n-->\r\n\r\n<included>\r\n  <property name='root.level' value='WARN'/>\r\n  <logger name='bar' level='INFO'/>\r\n  <logger name='foo' level='ERROR'/>\r\n</included>\r\n"

Since the above test is converting these into XMLs and comparing them as Strings, it is failing. Since the order of elements in an XML matters, converting these Strings into XMLs will not fix the test.

Fix

In order to make the ordering of loggerLevels deterministic, this fix is converting it's type from HashMap.java to LinkedHashMap.java, which retains insertion order. Furthermore this fix also changes the ordering of loglevels in getExpectedXml(boolean afterReset) in order to maintain a consistent loggerLevel ordering with the tests in the class.

I would love to get feedback on this Pull Request. Do let me know if you'd like to see any more changes!

Thanks

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.

1 participant