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

Test Adapter Improvements - Register test results incrementally #1430

Closed
wants to merge 13 commits into from
Closed

Test Adapter Improvements - Register test results incrementally #1430

wants to merge 13 commits into from

Conversation

ozyx
Copy link
Contributor

@ozyx ozyx commented Dec 6, 2016

Hello,
In this set of changes I have modified TestExecutor and run_tests.js to be able to communicate with each other via TestEvent objects. This is to allow the incremental logging of test results and to allow us to accurately record test duration by emitting test start events (or other events).

The overall goal of the changes in this branch is to fix issues #518 and #1157 (test efficiency and Mocha running before() and after() hooks multiple times per test suite.)

We have been using a version of these changes internally for the past month (running Mocha tests on TFS 2015 build definition) and... so far, so good. At this point I believe it is necessary to get a second pair of eyes on the code since fixing the Test Adapter issues required such a significant code change. I've been working with @mjbvz on this for the past couple of months (not sure if he is still working on nodejstools or not?) and through his guidance led the development of these changes.

Testing

More extensive testing is certainly needed, but here is a summary of some of the cases I've tested and their current statuses. Text highlighted in bold are issues that need investigation/fixes.

TestCase Mocha v2 Mocha v3 ExportRunner Tape
Basic passing test Passes Passes Passes Passes
Basic failing test Fails Fails Fails Passes -- not a regression, see #1429
Run All Tests Works as expected Works as expected Works as expected Runs all tests but tests always pass.. not a regression-- see #1429
Run Selected Tests Works as expected Works as expected Works as expected Runs only selected tests but tests always pass
Multiple tests in multiple files Works as expected Works as expected Works as expected Runs tests but tests always pass
Tests that write to stdout/stderr Logs are preserved in test result data '' '' ''
Long Running Tests Works as expected '' '' ''
Tests with RegExp characters in the title Test fails prior to being run '' '' ''
Tests with odd characters in the title Test fails prior to being run '' '' ''
Multiple test frameworks in a single project Works as expected '' '' ''
Tests with the same name Displays exception '' Restricted by compiler Registers only one test without displaying exception
...

Please let me know if there are any areas of the code you would like me to elaborate on, or any improvements necessary in order to get this code up to snuff for merging.

@msftclas
Copy link

msftclas commented Dec 6, 2016

Hi @ozyx, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

@billti
Copy link
Member

billti commented Apr 12, 2017

@ozyx can you take a look at #1524 . As the branches were stale compared to master, I did some merging and fixed some conflicts in a new branch. It should contain the above, and the prior work on the test-adapter-improvements branch.

@ozyx
Copy link
Contributor Author

ozyx commented May 4, 2017

Closing -- see #1524

@ozyx ozyx closed this May 4, 2017
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 this pull request may close these issues.

3 participants