Add support for Tape test framework #989
Conversation
|
Hi @jcansdale, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
|
Oops, I forgot to add the tape.js test framework to the installer. I'll update my branch and report back. |
…ugh to include contents of directory (i.e. 'tape.js')?
|
Added 'TestFrameworks\Tape' folder to Wix installer file. Is this enough to include the contained 'tape.js' file? Please check this because I haven't been able to build the installer locally! |
|
Hi @jcansdale, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
| @@ -0,0 +1,75 @@ | |||
| var fs = require('fs'); | |||
There was a problem hiding this comment.
[nit] I'm not sure our other test frameworks do this, but please add "use strict"; for this new file.
|
Thanks for looking into this! Just two minor comments and then we can merge the change in. We'll try to get this into the next NTVS 1.2 release if possible. |
When loading test script throws, output error and continue loading other scripts. Use strict.
|
Thanks for the code review! I've incorporated your suggestions. |
| @@ -15,6 +15,7 @@ | |||
| <Directory Id="Dir_$(var.key)TestFrameworks" Name="TestFrameworks"> | |||
| <Directory Id="Dir_$(var.key)TestFrameworksExportRunner" Name="ExportRunner" /> | |||
| <Directory Id="Dir_$(var.key)TestFrameworksMocha" Name="Mocha" /> | |||
| <Directory Id="Dir_$(var.key)TestFrameworksTape" Name="Tape" /> | |||
There was a problem hiding this comment.
The directory is defined, but it's not being used anywhere. You'll also need to include a File Include in NodejsToolsFiles.proj, otherwise it won't get installed via the msi.
<File Include="TestFrameworks\tape\tape.js">
<InstallDirectory>TestFrameworksTape</InstallDirectory>
</File>There was a problem hiding this comment.
Also you mentioned you were unable to build the installer? What error were you running into?
There was a problem hiding this comment.
I've been able to get the installer built. I'm a PowerShell newb, but you have good documentation!
|
Hey @jcansdale , thanks for the contribution! We've had a lot of folks request tape support lately, so it's exciting to finally add support! Sorry for jumping into this a little late... left a few more comments. We're cutting it a bit close for the v1.2 Beta ship-train, but let's definitely aim to pull this into a subsequent v1.2 release. |
|
What happened to all of @mousetraps comments? I can't find them anywhere! 😕 |
|
Sorry, @mousetraps got flagged as a bot by github for some reason, which hides all of her comments and prs. We've contacted Github support to get this fixed as soon as possible. Everything should show up again after that. |
|
Had to send in some DNA, and the good news is that I've been declared human! All my comments should now be visible 😃 |
|
Congratulations on your machine like speed and efficiency, but I'm glad to hear you've been confirmed as human! 😉 |
Added InstallDirectory entry for 'tape.js'. Follow always use curly brackets convention.
|
@mousetraps I've implemented your various suggestions, including: Added Tape UnitTest file templates for JavaScript and TypeScript. If you have a chance to give it a try, I'd be interested to hear how you get on. |
| <ZipItem Include="Templates\Files\TapeUnitTest\UnitTest.js" /> | ||
| <ZipItem Include="Templates\Files\TapeUnitTest\UnitTest.vstemplate" /> | ||
| <ZipItem Include="Templates\Files\TypeScriptTapeUnitTest\UnitTest.vstemplate" /> | ||
| <ZipItem Include="Templates\Files\TypeScriptTapeUnitTest\UnitTest.ts" /> |
There was a problem hiding this comment.
One minor note which can be addressed later... we'll need to update the VS "15" template manifest to account for this change.
https://github.com/Microsoft/nodejstools/blob/master/Nodejs/Product/Nodejs/Templates/VS_Nodejs.item.vstman
EDIT: filed an issue here: #1027
|
👍 Nice work, thanks!! We'll merge this in and start testing... hopefully it'll make 1.2 Beta! 😃 |
Issue #986
Fix
This pull adds support for the Tape JavaScript test framework.
There are unit tests for this in a separate project:
https://github.com/jcansdale/TestFrameworksNTVS
It would be nice to integrate these at some point (they are in a .njsproj project). I don't know where it would make sense to put them? The test framework support is useful on its own though, so I've submitted this pull request without the tests.