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

Add default types for the test packages. #4698

Merged
merged 5 commits into from Jun 11, 2018

Conversation

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Jun 8, 2018

Looking more carefully at the test runs at https://travis-ci.org/jupyterlab/jupyterlab/jobs/389434330#L1745-L1746, I noticed that the tsconfig for the tests was missing some types to help compilation.

This fixes the errors in the test suite, and moves us to using ts-loader instead of awesome-ts-loader. ts-loader seems to play more nicely with other tools in the webpack ecosystem (like the caching, etc.), and doesn't seem to be any slower in running the CI tests anymore.

@saulshanabrook
Copy link
Member

@saulshanabrook saulshanabrook commented Jun 8, 2018

Maybe we have to add CheckerPlugin to the testing webpack config? s-panferov/awesome-typescript-loader#347 (comment) https://github.com/s-panferov/awesome-typescript-loader#configuration

// `CheckerPlugin` is optional. Use it if you want async error reporting.
// We need this plugin to detect a `--watch` mode. It may be removed later
// after https://github.com/webpack/webpack/issues/3460 will be resolved.
const { CheckerPlugin } = require('awesome-typescript-loader')

module.exports = {

  // Currently we need to add '.ts' to the resolve.extensions array.
  resolve: {
    extensions: ['.ts', '.tsx', '.js', '.jsx']
  },

  // Source maps support ('inline-source-map' also works)
  devtool: 'source-map',

  // Add the loader for .ts files.
  module: {
    rules: [
      {
        test: /\.tsx?$/,
        loader: 'awesome-typescript-loader'
      }
    ]
  },
  plugins: [
      new CheckerPlugin()
  ]
};

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 8, 2018

I think we already have the checker plugin in the webpack config, right?

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 8, 2018

I'm also experimenting with switching to ts-loader.

@saulshanabrook
Copy link
Member

@saulshanabrook saulshanabrook commented Jun 8, 2018

Ah yeah, I missed that. I think switching to ts-loader could make sense. The space is changing so fast!

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 8, 2018

@saulshanabrook - I think this is ready. You can verify that things are better by looking at the Travis log at https://travis-ci.org/jupyterlab/jupyterlab/jobs/389880193 and seeing that underneath "Type checking in progress", it always says no errors are found :).

@jasongrout jasongrout added this to the Beta 3 milestone Jun 8, 2018
Copy link
Member

@saulshanabrook saulshanabrook left a comment

I am just gonna clone this, introduce a type error in a test, and make sure they fail.

// We need this plugin to detect a `--watch` mode. It may be removed later
// after https://github.com/webpack/webpack/issues/3460 will be resolved.
var CheckerPlugin = require('awesome-typescript-loader').CheckerPlugin;
var ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
Copy link
Member

@saulshanabrook saulshanabrook Jun 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the fork version speeds things up a lot?

Copy link
Contributor Author

@jasongrout jasongrout Jun 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from plain ts-loader, yes. it's basically doing some of the same things that awesome-ts-loader was doing to speed things up.

{ test: /\.tsx?$/, use: [
{ loader: 'cache-loader' },
{ loader: 'thread-loader', options: {workers: threadCpus} },
{ loader: 'ts-loader', options: {happyPackMode: true} },
Copy link
Member

@saulshanabrook saulshanabrook Jun 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happyPackMode sounds fun :)

@saulshanabrook
Copy link
Member

@saulshanabrook saulshanabrook commented Jun 11, 2018

I added a failing type in a test file and the tests seem to go on....

yarn test
yarn run v1.7.0
$ lerna run test:firefox --scope "@jupyterlab/test-*" --concurrency 1 --stream
lerna info version 2.9.0
lerna info versioning independent
lerna info scope @jupyterlab/test-*
@jupyterlab/test-application: $ python run-test.py --browsers=Firefox karma.conf.js
@jupyterlab/test-application: [I 13:13:55.330 KarmaTestApp] Writing notebook server cookie secret to /var/folders/m7/t8dvwtnn32z84333p845tly40000gn/T/tmp4rb3j44p/jupyter_runtime/notebook_cookie_secret
@jupyterlab/test-application: [I 13:13:55.354 KarmaTestApp] The port 8888 is already in use, trying another port.
@jupyterlab/test-application: [I 13:13:55.367 KarmaTestApp] Serving notebooks from local directory: /var/folders/m7/t8dvwtnn32z84333p845tly40000gn/T/mock_contents7eqrm53o
@jupyterlab/test-application: [I 13:13:55.367 KarmaTestApp] 0 active kernels
@jupyterlab/test-application: [I 13:13:55.367 KarmaTestApp] The Jupyter Notebook is running at:
@jupyterlab/test-application: [I 13:13:55.367 KarmaTestApp] http://localhost:8889/?token=bf4a69db9b8835761fdde4085fad3161821baa38a2e5f38b
@jupyterlab/test-application: [I 13:13:55.367 KarmaTestApp] Use Control-C to stop this server and shut down all kernels (twice to skip confirmation).
@jupyterlab/test-application: [I 13:13:55.367 KarmaTestApp] Welcome to Project Jupyter! Explore the various tools available and their corresponding documentation. If you are interested in contributing to the platform, please visit the communityresources section at https://jupyter.org/community.html.
@jupyterlab/test-application: [C 13:13:55.368 KarmaTestApp] \n    \n    Copy/paste this URL into your browser when you connect for the first time,\n    to login with a token:\n        http://localhost:8889/?token=bf4a69db9b8835761fdde4085fad3161821baa38a2e5f38b&token=bf4a69db9b8835761fdde4085fad3161821baa38a2e5f38b
@jupyterlab/test-application: [I 13:13:55.370 KarmaTestApp] > karma start --browsers=Firefox karma.conf.js
@jupyterlab/test-application: Starting type checking service...
@jupyterlab/test-application: Using 2 workers with 2048MB memory limit
@jupyterlab/test-application: START:
@jupyterlab/test-application: Type checking in progress...
@jupyterlab/test-application: ERROR in /Users/saul/p/jupyterlab/tests/test-application/src/router.spec.ts(19,7):
@jupyterlab/test-application: TS2322: Type '"/"' is not assignable to type 'Int16Array'.
@jupyterlab/test-application: ERROR in /Users/saul/p/jupyterlab/tests/test-application/src/router.spec.ts(31,27):
@jupyterlab/test-application: TS2345: Argument of type '{ base: Int16Array; commands: CommandRegistry; }' is not assignable to parameter of type 'IOptions'.
@jupyterlab/test-application:   Types of property 'base' are incompatible.
@jupyterlab/test-application:     Type 'Int16Array' is not assignable to type 'string'.
@jupyterlab/test-application: Version: typescript 2.9.1
@jupyterlab/test-application: Time: 2929ms
@jupyterlab/test-application: WARNING in /Users/saul/p/jupyterlab/~/ansi_up/ansi_up.js
@jupyterlab/test-application: (Emitted value instead of an instance of Error) Cannot find SourceMap 'ansi_up.js.map': Error: Can't resolve './ansi_up.js.map' in '/Users/saul/p/jupyterlab/node_modules/ansi_up'
@jupyterlab/test-application:  @ /Users/saul/p/jupyterlab/packages/rendermime/lib/renderers.js 7:18-36
@jupyterlab/test-application:  @ /Users/saul/p/jupyterlab/packages/rendermime/lib/index.js
@jupyterlab/test-application:  @ /Users/saul/p/jupyterlab/packages/application/lib/mimerenderers.js
@jupyterlab/test-application:  @ /Users/saul/p/jupyterlab/packages/application/lib/index.js
@jupyterlab/test-application:  @ ./src/shell.spec.ts
@jupyterlab/test-application: 11 06 2018 13:14:03.656:INFO [karma]: Karma v1.7.1 server started at http://0.0.0.0:9876/
@jupyterlab/test-application: 11 06 2018 13:14:03.658:INFO [launcher]: Launching browser Firefox with unlimited concurrency
@jupyterlab/test-application: 11 06 2018 13:14:03.689:INFO [launcher]: Starting browser Firefox
@jupyterlab/test-application: 11 06 2018 13:14:05.349:INFO [Firefox 60.0.0 (Mac OS X 10.13.0)]: Connected on socket HiX-HSnssQoSAsIIAAAA with id 45577363
@jupyterlab/test-application:   apputils
@jupyterlab/test-application:     LayoutRestorer
@jupyterlab/test-application:       #constructor()
@jupyterlab/test-application:         ✔ should construct a new layout restorer
@jupyterlab/test-application:       #restored
@jupyterlab/test-application:         ✔ should be a promise available right away
@jupyterlab/test-application:         ✔ should resolve when restorer is done
@jupyterlab/test-application:       #add()
@jupyterlab/test-application:         ✔ should add a widget to be tracked by the restorer
@jupyterlab/test-application:       #fetch()
@jupyterlab/test-application:         ✔ should always return a value
@jupyterlab/test-application:         ✔ should fetch saved data
@jupyterlab/test-application:       #restore()
@jupyterlab/test-application:         ✔ should restore the widgets in a tracker
@jupyterlab/test-application: WARN: 'save() was called prematurely.'
@jupyterlab/test-application:       #save()
@jupyterlab/test-application:         ✔ should not run before `first` promise
@jupyterlab/test-application:         ✔ should save data
@jupyterlab/test-application:     Router
@jupyterlab/test-application:       #constructor()
@jupyterlab/test-application:         ✔ should construct a new router
@jupyterlab/test-application:       #base
@jupyterlab/test-application:         ✔ should be the base URL of the application
@jupyterlab/test-application:       #commands
@jupyterlab/test-application:         ✔ should be the command registry used by the router
@jupyterlab/test-application:       #current
@jupyterlab/test-application:         ✔ should return the current window location as an object
@jupyterlab/test-application:       #routed
@jupyterlab/test-application:         ✔ should emit a signal when a path is routed
@jupyterlab/test-application:       #stop
@jupyterlab/test-application:         ✔ should be a unique token
@jupyterlab/test-application: LOG: 'Routing context.html was short-circuited by c'
@jupyterlab/test-application:         ✔ should stop routing if returned by a routed command
@jupyterlab/test-application:       #navigate()
@jupyterlab/test-application:         ✔ cannot be tested since changing location is a security risk
@jupyterlab/test-application:       #register()
@jupyterlab/test-application:         ✔ should register a command with a route pattern
@jupyterlab/test-application:       #route()
@jupyterlab/test-application:         ✔ should route the location to a command
@jupyterlab/test-application:   ApplicationShell
@jupyterlab/test-application:     #constructor()
@jupyterlab/test-application:       ✔ should create an ApplicationShell instance
@jupyterlab/test-application:     #leftCollapsed
@jupyterlab/test-application:       ✔ should return whether the left area is collapsed
@jupyterlab/test-application:     #rightCollapsed
@jupyterlab/test-application:       ✔ should return whether the right area is collapsed
@jupyterlab/test-application:     #currentWidget
@jupyterlab/test-application:       ✔ should be the current widget in the shell main area
@jupyterlab/test-application:     #isEmpty()
@jupyterlab/test-application:       ✔ should test whether the main area is empty
@jupyterlab/test-application:       ✔ should test whether the top area is empty
@jupyterlab/test-application:       ✔ should test whether the left area is empty
@jupyterlab/test-application:       ✔ should test whether the right area is empty
@jupyterlab/test-application:     #restored
@jupyterlab/test-application:       ✔ should resolve when the app is restored for the first time
@jupyterlab/test-application:     #addToTopArea()
@jupyterlab/test-application:       ✔ should add a widget to the top area
@jupyterlab/test-application: ERROR: 'Widgets added to app shell must have unique id property.'
@jupyterlab/test-application:       ✔ should be a no-op if the widget has no id
@jupyterlab/test-application:       ✔ should accept options
@jupyterlab/test-application:     #addToLeftArea()
@jupyterlab/test-application:       ✔ should add a widget to the left area
@jupyterlab/test-application: ERROR: 'Widgets added to app shell must have unique id property.'
@jupyterlab/test-application:       ✔ should be a no-op if the widget has no id
@jupyterlab/test-application:       ✔ should accept options
@jupyterlab/test-application:     #addToRightArea()
@jupyterlab/test-application:       ✔ should add a widget to the right area
@jupyterlab/test-application: ERROR: 'Widgets added to app shell must have unique id property.'
@jupyterlab/test-application:       ✔ should be a no-op if the widget has no id
@jupyterlab/test-application:       ✔ should accept options
@jupyterlab/test-application:     #addToMainArea()
@jupyterlab/test-application:       ✔ should add a widget to the main area
@jupyterlab/test-application: ERROR: 'Widgets added to app shell must have unique id property.'
@jupyterlab/test-application:       ✔ should be a no-op if the widget has no id
@jupyterlab/test-application:     #activateById()
@jupyterlab/test-application:       ✔ should activate a widget in the left area
@jupyterlab/test-application:       ✔ should be a no-op if the widget is not in the left area
@jupyterlab/test-application:       ✔ should activate a widget in the right area
@jupyterlab/test-application:       ✔ should be a no-op if the widget is not in the right area
@jupyterlab/test-application:       ✔ should activate a widget in the main area
@jupyterlab/test-application:       ✔ should be a no-op if the widget is not in the main area
@jupyterlab/test-application:     #collapseLeft()
@jupyterlab/test-application:       ✔ should collapse all widgets in the left area
@jupyterlab/test-application:     #collapseRight()
@jupyterlab/test-application:       ✔ should collapse all widgets in the right area
@jupyterlab/test-application:     #expandLeft()
@jupyterlab/test-application:       ✔ should expand the most recently used widget
@jupyterlab/test-application:       ✔ should expand the first widget if none have been activated
@jupyterlab/test-application:     #expandRight()
@jupyterlab/test-application:       ✔ should expand the most recently used widget
@jupyterlab/test-application:       ✔ should expand the first widget if none have been activated
@jupyterlab/test-application:     #closeAll()
@jupyterlab/test-application:       ✔ should close all of the widgets in the main area
@jupyterlab/test-application:     #saveLayout
@jupyterlab/test-application:       ✔ should save the layout of the shell
@jupyterlab/test-application:     #restoreLayout
@jupyterlab/test-application:       ✔ should restore the layout of the shell
@jupyterlab/test-application: Finished in 1.151 secs / 0.115 secs
@jupyterlab/test-application: SUMMARY:
@jupyterlab/test-application: ✔ 54 tests completed
@jupyterlab/test-apputils: $ python run-test.py --browsers=Firefox karma.conf.js
@jupyterlab/test-apputils: [I 13:14:10.543 KarmaTestApp] Writing notebook server cookie secret to /var/folders/m7/t8dvwtnn32z84333p845tly40000gn/T/tmpexlvffb4/jupyter_runtime/notebook_cookie_secret

@saulshanabrook
Copy link
Member

@saulshanabrook saulshanabrook commented Jun 11, 2018

I am trying out the webpack plugin here: https://gist.github.com/mvgijssel/d3b121ad50e34c09a124 to see if it helps, but so far things are still exiting cleanly and tests are running when there are type errors

@saulshanabrook
Copy link
Member

@saulshanabrook saulshanabrook commented Jun 11, 2018

I opened #4710 to track this problem where the tests still pass. Merging this, since at least the failures are more obvious now in the logs.

@saulshanabrook saulshanabrook merged commit 0810aab into jupyterlab:master Jun 11, 2018
2 checks passed
@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 11, 2018

Merging this, since at least the failures are more obvious now in the logs.

And it does fix the existing failures :)

@jasongrout jasongrout deleted the testtypes branch Jun 11, 2018
@jasongrout jasongrout mentioned this pull request Jun 12, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Aug 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants