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

Projects
None yet
2 participants
@yvanzo
Copy link
Contributor

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 some commits Feb 5, 2017

y-van-z
Turn URL cleanup tape subtests into tape tests
Workaround for tape subtests that blow call stack up in browser (FF)
@mwiencek

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Member

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

1 check passed

Jenkins Build finished.
Details

@yvanzo yvanzo deleted the yvanzo:fix-tests-for-browser branch Apr 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.