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

Clean up npm scripts and start running tests #725

Merged

Conversation

DustinCampbell
Copy link
Member

NPM script clean up

Since the repo started, we've had a single NPM script that ran on postinstall. All this did was run the VS Code install script and then launch tsc. Now, postinstall just runs the VS Code install script. The idea here is that install should really just lay down dependencies, not compile code. Compilation is split off into a separate script.

Here are the new npm scripts:

  • postinstall: As described above, this now just runs the VS Code install script after npm install.
  • compile: Runs the VS Code compile script to transpile TypeScript to JavaScript. Run this with npm run compile.
  • watch: Runs the VS Code compile script with the '-watch' flag to transpile TypeScript to JavaScript, and then watch for file changes and incrementally compile. Run this with npm run watch.
  • test: Runs the tests with Mocha. Note: This will only run tests that are already transpiled to JavaScript. Run with npm test.

Unit tests

  • Unit tests can be written in the 'test' folder. See 'test/sanity.tests.ts' for an example.
  • The launch.json has been updated to allow debugging of unit tests with the "Launch Tests" configuration.
  • The "test" task is now configured so that you can use the "Tasks: Run Test Task" command in VS Code.

Fixing some settings

  • Whitespace is a bit out of control. I've made an executable decision that the repo should have spaces with an indent size of 4. This is now added to the settings.json.
  • The 'out' folder is now excluded in settings.json so you won't see it show up in your directory tree.
  • The 'node_modules' folder is now excluded from search. This has been bothering me for awhile.

@DustinCampbell DustinCampbell added this to the 1.5 milestone Aug 27, 2016
@DustinCampbell
Copy link
Member Author

cc @gregg-miskelly, @chuckries

I'm not intending to merge this until after we ship 1.4, but wanted to get this out there to get your thoughts.

@ivanz
Copy link
Contributor

ivanz commented Aug 28, 2016

typings folder shouldn't be in source control - you can add typings install to the postinstall and include the typings.json?

@ivanz
Copy link
Contributor

ivanz commented Aug 28, 2016

Whitespace is a bit out of control. I've made an executable decision that the repo should have spaces with an indent size of 4. This is now added to the settings.json.

I suggest adding an .editorconfig file too (info)

@ivanz
Copy link
Contributor

ivanz commented Aug 28, 2016

I also vote for chai.js as the assert library of choice (if a vote is possible 😄 ). It has a solid ecosystem around it (helpers and libraries, etc) and enables to use both assert, should and expect type of syntax.

@DustinCampbell
Copy link
Member Author

typings folder shouldn't be in source control - you can add typings install to the postinstall and include the typings.json?

I was following the model of VS Code itself, which includes the typings folder, and Go extension, which also includes the typings folder. At the very least, the typings folder is needed to provide links to VS Code's typings which are deeper within node_modules. Note that VS Code's extension scaffolding support also creates a typings folder: https://code.visualstudio.com/docs/extensions/example-hello-world.

@DustinCampbell
Copy link
Member Author

I suggest adding an .editorconfig

Good idea -- will do!

@DustinCampbell
Copy link
Member Author

I also vote for chai.js as the assert library of choice (if a vote is possible 😄 ). It has a solid ecosystem around it (helpers and libraries, etc) and enables to use both assert, should and expect type of syntax.

Yup, I was planning to use chai as well.

@chuckries
Copy link
Contributor

Is it possible to have tsc run automatically as part of the f5 scenario for the extension? I know the newly proposed npm run watch will handle this, but I have often forgotten to start tsc -w and have been bitten by stale code several times.

@DustinCampbell
Copy link
Member Author

It's certainly possible. We just need a gulp task that does the compilation. I actually went down that path for awhile, but it requires further refactoring. The problem is that our gulpfile.js depends on compilation already having loaded because it expects coreclr-debug and omnisharp modules to already be present.

@DustinCampbell DustinCampbell deleted the clean-up-and-unit-tests branch September 1, 2016 15:09
@DustinCampbell DustinCampbell restored the clean-up-and-unit-tests branch September 1, 2016 15:09
@DustinCampbell DustinCampbell reopened this Sep 1, 2016
Since the repo started, we've had a single NPM script that ran on postinstall. All this did was run the VS Code install
script and then launch tsc. Now, postinstall just runs the VS Code install script. The idea here is that install should
really just lay down dependencies, not compile code. Compilation is split off into a separate script.

Here are the new npm scripts:

* `postinstall`: As described above, this now just runs the VS Code install script after `npm install`.
* `compile`: Runs the VS Code compile script to transpile TypeScript to JavaScript. Run this with `npm run compile`.
* `watch`: Runs the VS Code compile script with the '-watch' flag to transpile TypeScript to JavaScript, and then
watch for file changes and incrementally compile. Run this with `npm run watch`.
* `test`: Runs the tests with Mocha. Note: This will only run tests that are already transpiled to JavaScript. Run with
`npm test`.

* Unit tests can be written in the 'test' folder. See 'test/sanity.tests.ts' for an example.
* The launch.json has been updated to allow debugging of unit tests with the "Launch Tests" configuration.
* The "test" task is now configured so that you can use the "Tasks: Run Test Task" command in VS Code.

* Whitespace is a bit out of control. I've made an executable decision that the repo should have spaces with an indent
size of 4. This is now added to the settings.json.
* The 'out' folder is now excluded in settings.json so you won't see it show up in your directory tree.
* The 'node_modules' folder is now excluded from search. This has been bothering me for awhile.
@DustinCampbell
Copy link
Member Author

How are folks feeling about this? Shall I merge?

@gregg-miskelly
Copy link
Contributor

@DustinCampbell LGTM

@DustinCampbell DustinCampbell merged commit 3f50222 into dotnet:master Sep 2, 2016
@DustinCampbell DustinCampbell deleted the clean-up-and-unit-tests branch October 25, 2016 16:27
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.

5 participants