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

Add an Assert.Inconclusive method #169

Closed
Daniel-Tr opened this issue Nov 22, 2016 · 4 comments
Closed

Add an Assert.Inconclusive method #169

Daniel-Tr opened this issue Nov 22, 2016 · 4 comments

Comments

@Daniel-Tr
Copy link
Contributor

Hi,

have you ever considered to include an Assert.Inconclusive method, known from .Net (see https://msdn.microsoft.com/de-de/library/ms243815.aspx ), maybe raising some kind of an EInconclusiveTest exception?

This could replace/extend the ENoAssertionsMade exception and be handled equally (in first place).
On second hand, you could consider to turn FFailsOnNoAsserts into a member of type TTestResultType and handle such an exception as the defined result.

At last, one could consider to extend TTestResultType by an Inconclusive state (and let the ITestRunner and ITestLogger handle this case).

Possible use case: We have tests that we know won't work at specific operating systems or have other dependencies (like 'need a floppy drive for this test'). We neither wan't such a test be marked as failed nor passed, because it actually wasn't executed. But an information about it not being executed would be valuable.

@vincentparrett
Copy link
Member

I'm open to this suggestion, but any implementation would need to keep in mind that we would like to implement parrallel test runs at some point in time.

@rmcginty
Copy link
Contributor

rmcginty commented Apr 4, 2017

Here's the English link (above is in German): https://msdn.microsoft.com/en-us/library/ms243815.aspx

I think we could actually implement this fairly easily if we fully implement warning support. I did a quick search for TTestResultType.Warning, and I believe the GUI is the only thing that uses it (to warn if no asserts, rather than fail - which should be optional as discussed in #154).

Now, I don't want to get into "this test failing should be a warning" or anything like that - but I think making FailOnNoAsserts an optional warning, then add Assert.Inconclusive to this, and it would integrate nicely.

I too have a few tests that only work when certain conditionals are in place, not to mention, like MS says in the article above, it is the default stub for a new test - so the IDE expert could use it in the default code template.

I guess the only problem with this I see is something @vincentparrett or @staticcat needs to weigh in on: what about support for warnings on Continua and others that rely on NUnit output? Does NUnit output support any more states than DUnitX and Continua are supporting now? Looking at my CI, it is Pass/Fail/Not Run. I don't really feel the warning fits into any of those, and I wouldn't want to implement this if it couldn't flow across the board in a first-class implementation.

@vincentparrett
Copy link
Member

I'm not against this. DUnitX currently doesn't do anything with warnings either. Happy to accept a PR for both (makes sense to do them together).

NUnit supports Warnings and Inconclusive, so Continua CI should too, we'll deal with any issues around that if needed.

@vincentparrett
Copy link
Member

No PR eventuated so closing. Feel free to submit one for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants