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

Fix running tests in browser #477

Merged
merged 2 commits into from Mar 2, 2017

Conversation

yvanzo
Copy link
Contributor

@yvanzo yvanzo commented Feb 27, 2017

Fix two small bugs while running tests in browser, as reported in #449 (comment):

  1. too much recursion error caused by tests nesting, resolved by splitting URL Cleanup test (currently composed of subtests) into a myriad of tests
  2. Label guess case test failure using Turkish mode, resolved by initializing user settings as necessary

y-van-z added 2 commits February 27, 2017 18:21
Workaround for tape subtests that blow call stack up in browser (FF)
@mwiencek
Copy link
Member

mwiencek commented Mar 2, 2017

Ah, I see the guess case one is caused by changing the settings in the UI outside of the tests (because they share cookies, of course).

I guess Chrome and my version of Firefox both have higher recursion limits, though. :) Does it say in the console where in the code it's looping? Because if these simple/singly-nested tests are blowing up, I'd like to know if we're using the tape library incorrectly somehow.

@yvanzo
Copy link
Contributor Author

yvanzo commented Mar 2, 2017

Indeed, tests fail with Firefox/Archlinux whereas they pass with any Blink-based browser.

It is not looping. It can either stop on too much recursion error or Stack overflow error.
Where in the code depends on compiled scripts and browser usage.

IMHO, it is still an issue of JavaScript implementation of tape for nested tests. I cannot find the link again, but I recall a similar issue for a large number of (not-nested) tests which has been fixed a long time ago.

@mwiencek
Copy link
Member

mwiencek commented Mar 2, 2017

Sorry, looping was a poor word choice. I'm able to reproduce something with node --stack-size=500 root/static/scripts/tests/node-runner.js but that seems like a very low limit for a web browser and I can't make sense of it right away.

@mwiencek mwiencek merged commit c8d182f into metabrainz:master Mar 2, 2017
@yvanzo yvanzo deleted the fix-tests-for-browser branch April 2, 2017 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants