-
Notifications
You must be signed in to change notification settings - Fork 152
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
Update --labels switch with option to show real-time pass/fail results in console runner #24
Comments
@constructor-igor Now that the console is in another repo, you may need to start over. :-( |
After split of nunit repository, I can build nunit. but still cannot build nunit-console ;( Reference Mono.Cecil also has implementation of "ExtensionAttribute": So, I cannot understand why new cloned projects have different behavior on different computers. |
This is a well-known problem and I've "fixed" it a few times already. Our addition of Mono.Cecil exacerbates the problem. In the past, this normally only came up in our test assemblies - where we use .NET 3.5 for the test but reference a .NET 2.0 assembly. Try removing the attribute definition from nunit.engine assembly. If that fails, just turn off warnings as errors for now. |
After several attempts, I turned off "warnings as errors". Probably, file BUILDING.md contains obsolete options list: |
I have update BUILDING.md, it is out of date. |
There are issues in both repos RE BUILDING.md. |
I investigated suggested sequence and code (
I has renamed "All" to "After" and now I want to add "status". [TestCaseSource("EventData")]
public void EventsWriteExpectedOutput(string report, string labels, string expected)
{
var handler = new TestEventHandler(_writer, labels);
handler.OnTestEvent(report);
if (Environment.NewLine != "\r\n")
expected = expected.Replace("\r\n", Environment.NewLine);
Assert.That(Output, Is.EqualTo(expected));
} |
@constructor-igor I'm driving home from my trip today and can't look at code. I can give you some help this evening (in about 12 hours) or perhaps @rprouse can chime in with advice. |
@constructor-igor I'm sorry that this fell through the cracks. Here are a couple of test cases to get you started... new TestCaseData("<test-case fullname='SomeName' result='Passed' />", "After", "PASSED => SomeName\r\n"),
new TestCaseData("<test-case fullname='SomeName' result='Failed' />, "After", "FAILED => SomeName\r\n"), We probably need to work out a few things before going much further than that:
Once you get this basically working, I think you should do a PR. It won't be ready to merge yet but having a PR will make it easier to give line by line comments and should make me quicker to respond. :-) |
@constructor-igor How's this going - we are about to do a 3.5 release and I'm wondering if this will make it or if we should postpone. |
Hi Charlie, I am going to return to nunit activities this week. thank you, On Mon, Sep 26, 2016 at 9:46 PM, CharliePoole notifications@github.com
|
We're aiming to do it in stages, starting at the end of the week. The framework will be first. |
update:
Now I am going to add "Before" status, including tests. Current questions:
suggested tasks list:
added tests:
|
I add the samples for my better understanding 'Before' (not implemented) new TestCaseData("<test-case fullname='SomeName' result='Passed'/>", "Before", "=> SomeName\r\nPASSED => SomeName\r\n"),
new TestCaseData("<test-suite fullname='SomeName' result='Passed'/>", "Before", "=> SomeName\r\nPASSED => SomeName\r\n"), |
@constructor-igor I think it would be easier to review this and possibly make suggestions if you created a PR. Creating a PR doesn't necessarily mean your work is ready to merge. We do it all the time for anything that requires some degree of collaborative review because it enables making comments on specific lines. Just put a line saying Do Not Merge at the top of the description. |
created PR #78 |
Just a note, this is currently in the 3.6 milestone but we are close to doing the release. We can include this if it gets done before @CharliePoole or I goes to do the release, but if it is still outstanding we will move it out of the milestone. I am not saying this to rush anyone, just letting people know this may not be included in 3.6 😄 |
@rprouse @constructor-igor Since this enhancement is not making it into 3.5, I'm inclined to do a quick fix to the existing bug where "All" is being displayed when the test ends rather than when it starts. What do you think? |
@CharliePoole yes, if we cannot complete the feature right now, better to fix bug with "All". |
@CharliePoole I am okay with the quick fix for All, probably similar to what we added to dotnet-test-nunit. @constructor-igor feel free to carry on with this PR, we will get it in the next release. |
FYI, the fix is in. |
@dybs commented on Fri Aug 12 2016
When running a large number of test cases, it would be nice to see the pass/fail status of each case in the console as it completes. Using --labels=All, I can see which tests have run, but the results are not available until all cases have completed. Seeing which cases fail during a long scenario would allow me to investigate the failures while the remaining tests continue to run. It also has the advantage of knowing which cases passed/failed if the test scenario happens to crash and doesn't get to write the result file.
Per the discussion in #1735 some ideas are to modify --labels with the following options:
@CharliePoole commented on Fri Aug 12 2016
I like this idea, but I would modify/simplify it by preference:
Note that All is described as it is supposed to work already. There's currently a bug in that it runs at the end of a test. We should fix that. We should keep All so as not to break existing usage.
The introduction of After means that we have to treat it as also meaning On. That is, if a test produces output, there should be a label before that output and another one after it, together with the test status..
The change should be made in both the console runner and nunitlite. Once we split the repos, this issue will need to be duplicated.
The fix is relatively easy. Depending on who takes it, I can mentor them as needed.
@constructor-igor commented on Sun Aug 14 2016
Hi @CharliePoole,
Probably, I can start to work on the issue.
I checked all existing options of "--labels" parameter. Please, could you help me to understand suggested changes? I added sample with "--lablels=All". As I understand, the issue suggests to add "status" of each test?
thank you,
Igor.
@CharliePoole commented on Sun Aug 14 2016
Hi @constructor-igor - I've been meaning to poke you to see what you are up to. :-)
As you probably figured out, all the code is in Console's TestEventHandler and in NUnitLite's TestRunner. There are tests for TestEventHandler but not for TestRunner. :-(
I would first off fix the problem with All, which is supposed to be issued at the start of each test, not at the end. I think I broke this when I was fixing something else.
Once All is correct, you've got Before, since it's intended to work the same way.
After is the new thing, which is supposed to show status of the test. For that, I would use a combination of the
result
andlabel
attributes to display a set of values like...In NUnitLite, you'll have the ResultState available to use for equivalent logic.
@constructor-igor commented on Mon Aug 15 2016
I am going to start from "All".
But, right now, I don't understand expected behavior of "All".
So, when I'll investigate code I'll contact with you.
@CharliePoole commented on Mon Aug 15 2016
You might try this sequence - a bit different order from my first suggestion:
@constructor-igor commented on Mon Aug 15 2016
I try to build nunit by "build.cmd":
VS2013:
@CharliePoole commented on Mon Aug 15 2016
Are you using the latest master? Can you build in the IDE?
@constructor-igor commented on Mon Aug 15 2016
I cloned today and tried to build via build.cmd and IDE VS2013: same errors.
@CharliePoole commented on Mon Aug 15 2016
Using VS 2015?
@rprouse commented on Mon Aug 15 2016
I just got latest from master and ran
.\build.cmd
and it builds fine for me. Maybe do a full clean including deleting your packages folder and rebuild?@constructor-igor commented on Mon Aug 15 2016
I could build nunit solution on other computer.
I tried to build on my computer again (downloaded master to new folder) and see the error.
I investigate the issue and found, probably, same issues
I continue my investigation.
The text was updated successfully, but these errors were encountered: