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

Tinytest silently ignores all tests on uncaught errors #4979

Closed
dandv opened this Issue Aug 18, 2015 · 5 comments

Comments

Projects
None yet
7 participants
@dandv
Contributor

dandv commented Aug 18, 2015

_1 Upvote_

git clone git@github.com:iDoRecall/email-normalize.git
git checkout 64b18f371d8d3f7d62d6821a46d4ef62b752472d
cd email-normalize
meteor test-packages ./

Notice that apparently everything went ok. At a closer examination, only Server tests have been run, despite pacakge.js including the test on both client and server.

This is caused by the uncaught ReferenceError: Npm is not defined on the client, which halts the tests. Would it be possible to catch this type of case?

@anubhav94

This comment has been minimized.

Contributor

anubhav94 commented Aug 18, 2015

@dandv Thanks for catching this! This is a really good point and we'll definitely add it as a bug to track and fix in the future. Unfortunately, right now we're working on other projects and so as always, pull requests are welcome and encouraged!

@savvopoulos

This comment has been minimized.

savvopoulos commented Feb 8, 2016

+1. This seems like a serious problem, given that the point of tests is to catch errors like this.
I've been bitten by this many times.

How can we fix this? Happy to send a PR if you give me some pointers.

@Maxim-Filimonov

This comment has been minimized.

Maxim-Filimonov commented Mar 14, 2016

I did some preliminary investigation of the issue today. According to what I found error actually happens before tinytest kicks in.
Tinytest tests are injected a generated file "local-test__package_name_" created by the test runner. The file has a bunch of imports at the top including the import of a package under test which is where the exception occurs. Those imports are not wrapped into any error handling logic which causes interpolation of the whole file to fail and leads to tests not being executed.
The simplest thing, I can think of is to change generator of the file to wrap imports into error handling logic, then on failure create a single error which displays the exception.
Thoughts ?

@Maxim-Filimonov

This comment has been minimized.

Maxim-Filimonov commented Mar 22, 2016

Ok some after further investigation maybe someone from MDG can at least confirm the direction. Generation of the test file happens through standard linker and it doesn't look like there is an easy way to extend that generation without touching linker directly, which I would rather avoid.
I have double checked that tinytest ignores tests only when error occurs in client tests. On error during server tests standard meteor error handler kicks in. Based on that I'm thinking to just use window.onerror to catch an exception while tests are running and then to create a failure with that exception which will fail the build.

@zol zol removed the backlog label May 3, 2016

hwillson added a commit to hwillson/meteor that referenced this issue Aug 22, 2017

Visually notify of uncaught exceptions breaking client tests
These changes add a global `window.onerror` event handler
to the `test-in-browser` package, to catch any previously
uncaught exceptions. When uncaught exceptions are found,
an alert box is displayed above the test results table.
This should help notify developers of hidden uncaught
exceptions that could be preventing client tests from
running.

Fixes meteor#4979.
@hwillson

This comment has been minimized.

Member

hwillson commented Aug 22, 2017

PR #9034 should help with this:

screenshot 2017-08-22 09 24 27

benjamn added a commit that referenced this issue Aug 23, 2017

Visually notify of uncaught exceptions breaking client tests (#9034)
* Visually notify of uncaught exceptions breaking client tests

These changes add a global `window.onerror` event handler
to the `test-in-browser` package, to catch any previously
uncaught exceptions. When uncaught exceptions are found,
an alert box is displayed above the test results table.
This should help notify developers of hidden uncaught
exceptions that could be preventing client tests from
running.

Fixes #4979.

* Bump the test-in-browser package version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment