Jasmine tests are failing locally

[Flaburgan]

Despite the nice work done in #8333 my Jasmine tests are still failing locally, both with Firefox and Chromium.
Sometimes it gets redirected to another URL so it quits the tests:

2022-07-02.20-39-50.mp4

Sometimes it gets stuck because it asks if we're sure we want to reload the page.

@ragesoss do you reproduce those problems? Any idea what's going on?

[ragesoss]

@Flaburgan sorry, I don't know. I've never used this browser-based way of running Jasmine tests before; I've only run them via terminal.

[cmrd-senya]

Hmm, I just ran the tests both in Chromium and Firefox locally, and got the green build

@Flaburgan are there any errors in the browser console?

[cmrd-senya]

@Flaburgan could you try disabling this test https://github.com/diaspora/diaspora/blob/develop/spec/javascripts/app/views/conversations_inbox_view_spec.js#L88 ? Will it make the rest of the testsuite pass for you?

[Flaburgan]

@Flaburgan sorry, I don't know. I've never used this browser-based way of running Jasmine tests before; I've only run them via terminal.

@ragesoss Do you mind sharing the command you use to run them in the terminal?

[SuperTux88]

@Flaburgan I also only run jasmine in the terminal and never had problems with it. And the command for that is: RAILS_ENV=test bin/rake jasmine:ci

[Flaburgan]

When I run them this way, they hang even on the develop branch after:

.app.views.Bookmarklet prefills the publisher: passed
.app.views.Bookmarklet handles dirty input well: passed
.app.views.Bookmarklet allows changing a prefilled publisher: passed
.app.views.Bookmarklet keeps the publisher disabled after successful post creation: passed
.app.views.CommentMention initialize passes correct URL to PublisherMention contructor: passed
.app.views.CommentStream binds calls appendComment on insertion to the comments collection: passed

(output generated with the help of e3a4422)

I'm quite sure the browser is stuck on the "Are you sure you want to quit the page" pop ups, problem that I tried to solve (without success) with a517e66 and 9b39889

[Flaburgan]

For information, after pulling the latest changes in develop I don't reproduce locally anymore, even in my improve-report-form branch which is triggering the hang on the CI. @denschub spent quite some time investigating this yesterday and found that the guilty test is https://github.com/diaspora/diaspora/blob/develop/spec/javascripts/app/views/bookmarklet_view_spec.js#L46 which is creating a race condition. When this test is disabled, the CI is green again.

[denschub]

Note that I am not 100% certain. I only spotted this one event resulting in a navigation, but there might be others. I also don't quite understand why it fails. In theory, the Publisher should not trigger a navigation, and it should just XHR. And in theory, that XHR should be captured by jasmine-ajax. Other publisher tests use a spy'ed callback for the submit trigger, like

diaspora/spec/javascripts/app/views/publisher_view_spec.js

Lines 244 to 245 in d4f92a8

	var submitCallback = jasmine.createSpy().and.returnValue(false);
	form.submit(submitCallback);

but that didn't work in this case - it appears like passing a callback to the form submit here results in the publisher simply not working. I didn't dig deeper here, but this might be a possible approach to make this test work reliably. Note that there are also some other callback-less form submits, and I don't know if they are reliable or not.

It's probably worth noting that accessing /bookmarklet throws a JS exception, which I haven't looked into at all. It's possible that somehow, this exception also occurs inside the Jasmine run, and prevents some handlers from being set up, which then result in document navigation. But I don't know that for sure, this needs a lot more investigation.

I pushed a commit that disables the test in question directly into @Flaburgan's PR #8035, and I think it's fine shipping this. The disabled spec is only a UX-test, not directly a functional test.

Generally speaking, I feel like some of the Jasmine tests should be ported to Cucumber, or another test framework designed for full-depth integration tests. Technically, Jasmine was designed to be a unit test framework, and the fact that there are DOM mutations happening at all in our tests is a bit spooky to me. But I'm just noting this here to have a papertrail of that thought - I don't think I can personally spend the time on rewriting those tests, and I'm not expecting anyone else to do that, either. :)