jasmine / jasmine-browser-runner

Serve and run your Jasmine specs in a browser
49 stars 23 forks source link

runSpecs producing unexplained "cyclic object value" on Firefox #18

Closed chadlwilson closed 2 years ago

chadlwilson commented 2 years ago

Not sure if this is in the right place to report, but when running batch/CI mode with runSpecs on Firefox jasmine-browser-runner fails with

$ jasmine-browser-runner runSpecs --config=spec/javascripts/support/jasmine-browser.js
Jasmine server is running here: http://localhost:56034
Jasmine tests are here:         /Users/chad/Projects/community/gocd/gocd/server/src/main/webapp/WEB-INF/rails
Source files are here:          /Users/chad/Projects/community/gocd/gocd/server/src/main/webapp/WEB-INF/rails/public/assets
Running tests in the browser...
JavascriptError: Cyclic object value
    at Object.throwDecodedError (/Users/chad/Projects/community/gocd/gocd/server/src/main/webapp/WEB-INF/rails/node_modules/selenium-webdriver/lib/error.js:522:15)
    at parseHttpResponse (/Users/chad/Projects/community/gocd/gocd/server/src/main/webapp/WEB-INF/rails/node_modules/selenium-webdriver/lib/http.js:549:13)
    at Executor.execute (/Users/chad/Projects/community/gocd/gocd/server/src/main/webapp/WEB-INF/rails/node_modules/selenium-webdriver/lib/http.js:475:28)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async thenableWebDriverProxy.execute (/Users/chad/Projects/community/gocd/gocd/server/src/main/webapp/WEB-INF/rails/node_modules/selenium-webdriver/lib/webdriver.js:735:17) {
  remoteStacktrace: 'WebDriverError@chrome://remote/content/shared/webdriver/Errors.jsm:183:5\n' +
    'JavaScriptError@chrome://remote/content/shared/webdriver/Errors.jsm:362:5\n' +
    'evaluate.assertAcyclic@chrome://remote/content/marionette/evaluate.js:52:11\n' +
    'evaluate.toJSON@chrome://remote/content/marionette/evaluate.js:323:14\n' +
    'receiveMessage@chrome://remote/content/marionette/actors/MarionetteCommandsChild.jsm:177:31\n'
}
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

It seems to work perfectly fine with

I'm not sure what is different other than reporters, or how to debug further - any tips appreciated.

Environment

sgravrock commented 2 years ago

If I'm interpreting that stack trace right, Selenium is trying to send the result of a command from the browser to the driver, but the result fails to serialize because it contains a cycle. Assuming that the command is part of Jasmine and not internal to Selenium, this is the likely source. That code guards against cycles in the passed and failed expectations, but not elsewhere. I can get the same stack trace if I do something like this:

const name = {};
name.loop = name;

it(name, function() {});

I assume you're not doing anything like that, but maybe something about your test suite is causing a cycle to appear in an unexpected place. If I'm right, you might be able to find it with a custom reporter:

const reporter = {};

for (const method of ['jasmineStarted', 'jasmineDone', 'suiteStarted', 'suiteDone', 'specStarted', 'specDone']) {
  reporter[method] = function(result) {
    try {
      JSON.stringify(result);
    } catch (e) {
      console.error('Reporter method', method, 'got a cyclic result:', result);
    }
  };
}

jasmine.getEnv().addReporter(reporter);

Put that in a helper file, then:

  1. Make sure it's referenced by the helpers field in jasmine-browser.json.
  2. Run the serve subcommand.
  3. Visit the page in the browser and wait for specs to run.
  4. Look for errors in the console.

You might get some false positives, since that will flag cycles in expectation results even though those shouldn't cause the error you're getting.

chadlwilson commented 2 years ago

Thanks for your reply @sgravrock - I've tried that and I don't see any errors in the console. Also verified the helper/reporter suggested above is being loaded :-(

If it only fails on Firefox and not Chrome, does that point in any particular direction?

sgravrock commented 2 years ago

Hmm. I think that points a bit more at the possibility of an internal error in Selenium, but I don't have any idea what would cause that. If you can put together a repo that reproduces the error, I can try to isolate it further.

chadlwilson commented 2 years ago

After some brute-force trial and error I think the problem here is some issue with the Firefox geckodriver/Marionette impl and some dodgy old Javascript code we have under test which is hacking with toJSON and messing with the serialization/marshalling of results as JSON back to the browser runner via driver.executeScript(...).

At root I am trying to migrate an older rails/asset-pipeline project from the EOL Jasmine RubyGem onto jasmine-browser-runner. I now note that there was some dodgy monkey-patch for this problem in the old Ruby code.

Short of resolving the root problem, I guess I'll need to find a way to achieve the same monkey-patching within jasmine-browser-runner or find some other approach. 😢

sgravrock commented 2 years ago

Oof.

I think the change you want is, by itself, pretty simple. I think you want something like this untested patch:

diff --git a/lib/runner.js b/lib/runner.js
index cf4f385..2652d59 100644
--- a/lib/runner.js
+++ b/lib/runner.js
@@ -14,7 +14,7 @@ function getBatch(driver) {
       "try { JSON.stringify(expectation.actual); } catch (e) { expectation.actual = '<circular actual>'; }\n" +
       '}\n' +
       '}\n' +
-      'return results;'
+      'return JSON.parse(JSON.stringify(results));'
   );
 }

You could probably monkey patch that in if you were really determined, but you'd end up having to replace quite a bit of unrelated code to make that happen. It would probably be less work overall to maintain a fork of jasmine-browser-runner. Alternately I wouldn't be opposed to an extension point that allows this sort of thing to be patched in cleanly, although I'm not sure what that would look like.

Short of resolving the root problem

Prototype.js, right? I assume (a) that you know it has security holes that will likely never be fixed, and (b) that removing it would be really hard. Best of luck to you.

chadlwilson commented 2 years ago

Oof, indeed :-)

Prototype.js, right? I assume (a) that you know it has security holes that will likely never be fixed, and (b) that removing it would be really hard. Best of luck to you.

I wasn't specifically aware of this, as nothing is being reported against 1.6.0.3 from the various tools we're running but neither am I surprised given its age.

And yeah, although I don't have all the historical background and frontend is not my strong suit, I imagine the reason it is still there is due to the difficulty of removing it. Much of this "legacy" JS has been gradually rewritten into webpacked typescript SPAs alongside a move away from Rails (tested via Karma-jasmine), however there is still stuff left. Was seeing what I could do to keep the testing "lights on" as it were.

chadlwilson commented 2 years ago

Be afraid, be very afraid: https://github.com/chadlwilson/gocd/commit/0e395dcbfc2296af4536188f8aba5594b5228193

This will do for now; hacky and I dont really understand why it works, but still both a better experience than with the old Ruby Jasmine monkey-patch (I can get failure details back over the CLI just fine) and at least not using deprecated stuff.

Thanks for your help!