jupyter-widgets / ipywidgets

Interactive Widgets for the Jupyter Notebook
https://ipywidgets.readthedocs.io
BSD 3-Clause "New" or "Revised" License
3.16k stars 950 forks source link

issues with unit testing custom widgets #1390

Open rolandjitsu opened 7 years ago

rolandjitsu commented 7 years ago

I made a simple widget cookiecutter that uses TypeScript instead of JS and I added some tools to also run unit tests and linting, but I'm having some trouble unit testing the widgets.

If I import from ./example I get the following error:

 karma start karma.config.ts --browsers Chrome --single-run --log-level error

Chrome 58.0.3029 (Mac OS X 10.12.5) ERROR
  Uncaught ReferenceError: jQuery is not defined
  at /var/folders/k5/5f8qb7191cngz3v79ky_w5w0005njt/T/karma-typescript-bundle-10566ag6ZN8l7ET5x.js:14458

Which basically tells me that jQuery is accessed as a global var somewhere but it's undefined. I could actually find where it occurred:

if ( typeof define === "function" && define.amd ) {

        // AMD. Register as an anonymous module.
        define( [
            "jquery",
            "./mouse",
            "../keycode",
            "../version",
            "../widget"
        ], factory );
    } else {

        // Browser globals
        factory( jQuery ); // THIS IS WHERE IT BREAKS
    }

I tried to remove all UI widgets from the index.js file within jupyer-js-widgets package and I don't see the error anymore, which suggested that the UI widgets within the lib must require/import something that references jQuery as a global variable.

I also tried including jquery lib before I run all unit tests, but then I get another error:

Chrome 58.0.3029 (Mac OS X 10.12.5) ERROR
  Uncaught TypeError: Cannot read property 'mouse' of undefined
  at /var/folders/k5/5f8qb7191cngz3v79ky_w5w0005njt/T/karma-typescript-bundle-10687kQQBEOX5mCp5.js:13504

And that was due to $.ui.mouse, where $.ui seems to be undefined.

So I'm unsure how I could actually fix this.

jasongrout commented 7 years ago

For background, jquery is used in two ways:

  1. To provide the this.$el attributes for views for backwards compatibility. This is done in several files, such as widget_box, etc.

  2. To provide the slider, which is the jqueryui slider.

It looks like the actual lines quoted above come from dependencies, not directly from our code.

Our own test setup is at https://github.com/jupyter-widgets/ipywidgets/tree/master/jupyter-js-widgets/test - does it help to see how we set things up there?

rolandjitsu commented 7 years ago

@jasongrout thanks for the swift response. I actually already browsed through that and I couldn't figure out what is going on.

I mean, I have no extra dependencies, just jupyter-js-widgets. But I can confirm that removing some of the lines from jupyter-js-widgets/src/index.ts to jupyter-js-widgets/src/index.ts#L22 does make the error go away.

So the issue occurs somewhere among those lines. The problem might amongst these lines jupyter-js-widgets/src/widget_int.ts#L22, pulling in the jquery lib and or jquery ui.

I'm not certain though 😕

jasongrout commented 7 years ago

The lines you quoted above seem to implicate jqueryui.

I notice your webpack and karma configurations seem to be using typescript to compile to es6 and then compiling that to es5 using babel. Could you try having a much simpler configuration like we have for karma and webpack--straight typescript to es5?

rolandjitsu commented 7 years ago

@jasongrout definitely, I'll give it a try a bit later and let you know.

rolandjitsu commented 7 years ago

@jasongrout I actually forgot to mention that I run the tests directly through Karma and karma-typescript, so Webpack will never build anything. The assets are build on the fly, so maybe that could be the reason. Since webpack will actually add the referenced dependencies, whereas I'd need to add myself if I use the current setup with karma-typescript.

But I actually tried to include jquery and jquery ui, but I got the second error I describe. I'll have to debug, but it could help if you could use the same setup I use for unit tests (use only karma with karma-typescript) and see if you get the same error.

jasongrout commented 7 years ago

You're also using karma-typescript-es6-transform, right? (https://github.com/rolandjitsu/jupyter-widget-cookiecutter/blob/master/%7B%7Bcookiecutter.github_project_name%7D%7D/js/karma.config.ts#L22). Any chance you could make things simpler by not running it through babel?

rolandjitsu commented 7 years ago

If you check, I just pushed some changes and completely removed babel, same issue though.

jasongrout commented 7 years ago

Thanks. It would be interesting to me to try to iterate towards a common testing infrastructure, and that might uncover the issue. I'm out for a bit, but can look at this when I get back (and anyone else, feel free to take this up).