Report skipped Mocha tests as "Skipped"

[ozyx]

Issue #1555

Bug

Skipped Mocha tests using it.skip() are reported as "Failed" by TestExecutor

Fix

Use Mocha pending event to enable TestExecutor to report skipped Mocha tests as "Skipped"

Testing

Create some Mocha tests such as:

var assert = require('assert');
var mocha = require('mocha');

describe('Test Suite 1', function () {

    it.skip('SkipTest', function () {
        assert.ok(true, "This shouldn't fail");
    });

    it('NotSkipTest', function () {
        assert.ok(true, "This shouldn't fail");
    });

    it('FailTest', function () {
        assert.ok(false, "This should fail");
    });

    it.skip('AlsoSkipTest', function () {
        assert.ok(true, "This shouldn't fail");
    });
})

Run tests using TestExplorer (RunAll or individually)
Tests marked with it.skip() should be reported as "Skipped"

[fforjan]

@ozyx do you know where TestOutcome.Skipped is coming from ?
When I look at the MSDN, https://msdn.microsoft.com/en-us/library/microsoft.visualstudio.testtools.common.testoutcome.aspx, I wasn't able to find this specific enum value

[ozyx]

TestOutcome.Skipped is coming from Microsoft.VisualStudio.TestPlatform.ObjectModel.

[fforjan]

@paulvanbrenk @billti I was wondering, what is your expected timeline while answering to pull request like this from the community ? We've allocated some time to our team members, typically @ozyx previously to contribute and work with you but it's difficult to explain to our management team that, yes, the code we have contributed is pushed for more than a sprint and we can't claim that the code is merge and accepted by the Microsoft team.
What should we expect from you (as a project) ?

[paulvanbrenk]

First of all, I want to say thank you for your contributions, we do really appreciate them.

The reason we're hesitant in taking this particular PR, is that the testadapter is at a crossroads. One of our partner teams has taken the code, and they are in the process to make it work with other projects, e.g. ASP.NET and UWP, and such take it out of the NTVS package. While, at the same time we're stabilizing the 1.3 release. So, as you can imagine, we're a little hesitant to take any changes there.

So, long story short. This particular PR, happens to come at an unfortunate moment in the development cycle, but in general we should get back to you within the week.

[billti]

I'd also add that while appreciated and helpful, contributions are not "free" on this end. For example, the previous contribution you allude to above was great to get in the product, but it still took days of work to get the code tested and working in all required scenarios before I could merge it.

Accepting/merging any non-trivial change has a cost on the team, and thus if it's not a feature or fix we already had planned for the milestone, has to be prioritized with other work we already have on our plate.

Apologies for the delay. We'll try and look at it shortly.

[fforjan]

Thanks, it wasn't clear from your side of the status on this one.
Just a small word on the previous contribution, we work with you guys to address your feedback, it was initially started in October, last modification in December without feedback and merge recently....
Meaning It took 6 months for us to claim that work done- meaning we could have also help you to complete the work if it was requested on time :)

On our side we are interested to provide inputs, for those 2 contributions, there is one other defect under my name we will try to correct. But unfortunately, I cannot use this a good example about our contributions to this project :(

Let's say both of those are coming at the wrong time (the previous one before VS2017 stabilization) and this one before your 1.3 stabilization. Is there any 'windows' for stabilizing or when pull request will be in filter mode available somewhere ? Or is it something you could consider to make visible ?

PS: not trying to defend we did all good or saying you're doing the bad job, but as we did allocate resources on the previous pull request, I don't have a good track record a the moment :/

[billti]

Trailing whitespace on this line. Please remove.

[billti]

Remove trailing whitespace

[billti]

You don't need to report the 'pending' property here, it is unused. It is only used from the 'result' object.

[billti]

As above - 'pending' property is superfluous and unused. Please remove.

[billti]

A couple of minor changes, else looks good. Please port the the v1.3.x branch too.

Thanks!!

[ozyx]

@billti Awesome, thanks. Just a clarification -- by "port the v1.3.x branch" you mean request to merge these changes into that branch, as well? Just want to make sure before doing anything.

[billti]

We can do that tomorrow. No problem.

The 1.3 release we just did for VS 2015 was going to be our last. Are you guys on VS2017 yet, or still VS 2015. If you need this for VS 2015 we can spin one more official build with it.

[ozyx]

Yes the team is using VS2015 and will be for the foreseeable future, so getting a build with these changes included will be tremendously helpful. Thanks much!

[fforjan]

@billti @paulvanbrenk Thanks for the merge ! Next time you're in orange county, CA, beers are on me !

[paulvanbrenk]

I noticed you have created #1561 too, is this something you want to take a crack at?

I want to hold off on doing another build till sometime next week, to see if we have more critical feedback we should address.

Thanks!

[ozyx]

@paulvanbrenk Yes, if we could get that fix in the next build also that would be ideal. I've already started work on a potential fix, so I'll just open another PR when that's ready.

Thanks again for all your help.