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

False positives in try-catch blocks with Asserts revealed when using NUnit 3.10.0+ #301

Closed
NightOwl888 opened this issue Jun 30, 2020 · 0 comments · Fixed by #299
Closed
Assignees

Comments

@NightOwl888
Copy link
Contributor

Some of the test asserts are structured in a way that makes them potentially catch any exception that NUnit throws to implement its behavior. For example:

try
{
    // Test condition (supposed to throw)
    Assert.Fail("Did not throw");
}
catch (Exception)
{
    // ok
}

We were getting some false positives in NUnit 3.9.0 and prior, since in those cases the catch block would swallow the exception that NUnit throws as well as the intended exception. However, NUnit patched this behavior in 3.10.0 and now these tests are failing.

Upgrading to a newer version of NUnit requires us to fix these broken statements that are falling through. In addition, we should also change these to Assert.Throws or Assert.ThrowsAnyOf to make things easier to manage.

Fortunately, I have reviewed the issues and none of the problems affect end users, they were just problems with injecting mock settings during testing. This issue is closely related to #267.

@NightOwl888 NightOwl888 added this to the 4.8.0-beta00009 milestone Jun 30, 2020
@NightOwl888 NightOwl888 self-assigned this Jun 30, 2020
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Jun 30, 2020
…apter to 3.16.1, Microsoft.NET.Test.Sdk to 16.6.1 (apache#301)
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Jun 30, 2020
…ors(): Reflection doesn't throw exceptions when it cannot find a constructor in .NET, we need to test for null instead. (apache#301)
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Jun 30, 2020
… property that can be used to toggle "asserts" on and off in the release build, similar to how it works in Java. The setting can be injected by end users with the "assert" system property (which is a boolean). (apache#301)
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Jun 30, 2020
…apter to 3.16.1, Microsoft.NET.Test.Sdk to 16.6.1 (apache#301)
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Jun 30, 2020
…ors(): Reflection doesn't throw exceptions when it cannot find a constructor in .NET, we need to test for null instead. (apache#301)
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Jun 30, 2020
… property that can be used to toggle "asserts" on and off in the release build, similar to how it works in Java. The setting can be injected by end users with the "assert" system property (which is a boolean). (apache#301)
NightOwl888 added a commit that referenced this issue Jun 30, 2020
…apter to 3.16.1, Microsoft.NET.Test.Sdk to 16.6.1 (#301)
NightOwl888 added a commit that referenced this issue Jun 30, 2020
…ors(): Reflection doesn't throw exceptions when it cannot find a constructor in .NET, we need to test for null instead. (#301)
NightOwl888 added a commit that referenced this issue Jun 30, 2020
… property that can be used to toggle "asserts" on and off in the release build, similar to how it works in Java. The setting can be injected by end users with the "assert" system property (which is a boolean). (#301)
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 a pull request may close this issue.

1 participant