SpringAll icon indicating copy to clipboard operation
SpringAll copied to clipboard

Jasmine tests are failing locally

Open Flaburgan opened this issue 3 years ago • 8 comments

Despite the nice work done in https://github.com/diaspora/diaspora/pull/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:

https://user-images.githubusercontent.com/930064/177012700-20f2afa9-0046-41ae-9490-06c077bd9900.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?

Flaburgan avatar Jul 02 '22 18:07 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 avatar Jul 06 '22 16:07 ragesoss

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

@Flaburgan are there any errors in the browser console?

cmrd-senya avatar Jul 08 '22 19:07 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?

cmrd-senya avatar Jul 08 '22 20:07 cmrd-senya

@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?

Flaburgan avatar Jul 17 '22 21:07 Flaburgan

@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

SuperTux88 avatar Jul 17 '22 21:07 SuperTux88

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 https://github.com/diaspora/diaspora/pull/8035/commits/e3a4422f45c09d12a8a45f5a149a38a1a382a3a0)

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 https://github.com/diaspora/diaspora/pull/8035/commits/a517e667d7f1211aa9dbfec910fdc46af4a1c788 and https://github.com/diaspora/diaspora/pull/8035/commits/9b39889e4ae9389382a8f3f60255274edf0ac0ed

Flaburgan avatar Jul 17 '22 22:07 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.

Flaburgan avatar Jul 22 '22 09:07 Flaburgan

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

https://github.com/diaspora/diaspora/blob/d4f92a8fae2bcc0eb716622cb471276d38e8e305/spec/javascripts/app/views/publisher_view_spec.js#L244-L245

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. :)

denschub avatar Jul 22 '22 13:07 denschub