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

Lucene.Net.TestFramework: Improved error handling and test reporting #1092

Conversation

NightOwl888
Copy link
Contributor

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a change, please open an issue to discuss the change or find an existing issue.

Improved error handling and test reporting

Description

This fixes several concerns regarding output during testing:

  1. The details of the random test configuration are now output only if the test fails and will appear in the test message directly. This used to be where they appeared in older versions of NUnit and this restores that functionality.
  2. Exceptions that were being called in LuceneTestCase.OneTimeStartUp() and LuceneTestCase.OneTimeTearDown() were not very visible because NUnit doesn't fail the test when they occur there. This changes it from re-throwing exceptions with an additional message to logging to the output. It goes to stderr in all cases except in OneTimeTearDown() where something is redirecting it to stdout. Either way, this changes the ADO pipeline to fail the build so we can avoid shipping the test framework with bugs in these methods.
  3. Adds a system property named tests:failontestfixtureonetimesetuperror that can be used to cause the test framework to fail all of the tests when there is an error in OneTimeSetUp(). We may need to revisit this because there are asserts thrown here that users are supposed to see when dependency injection for codecs is not setup correctly.
  4. Removes all messages from the Assert.Pass() method because this bloats the size of the test logs with no benefit.

…Fixed error reporting, since NUnit swallows exceptions in these events. This will write them to stderr by default and allows enabling failing the tests if an exception occurs in LuceneTestCase.OneTimeSetUp() which will make them appear in the Test Output window in Visual Studio. The failures can be enabled by setting the tests:failontestfixtureonetimesetuperror system property to true.
…ment variable (defaulting to 'true') to set the system property of the same name.
…ic.Dictionary<TKey, TValue>, which will format itself. Marked fields private instead of internal.
… from SetUp() to TearDown() and only include them in the test message upon failure.
… messages from calls to Assert.Pass(), since this writes to the output and bloats the size of the test logs with no benefit.
… when maximumAllowedFailures is 0 and there are errors in the stderr file.
… when maximumAllowedFailures is 0 and there are errors in the stdout in the TestResults.trx file.
@NightOwl888 NightOwl888 added the notes:improvement An enhancement to an existing feature label Jan 8, 2025
@NightOwl888 NightOwl888 added this to the 4.8.0-beta00018 milestone Jan 8, 2025
@NightOwl888 NightOwl888 requested a review from paulirwin January 8, 2025 12:39
Copy link
Contributor

@paulirwin paulirwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Happy to see the log noise reduced.

@NightOwl888 NightOwl888 merged commit a9f7d30 into apache:master Jan 8, 2025
265 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes:improvement An enhancement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants