892048 - [test-agent] The test agent hangs when 1 test run is asked but another is already running

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::TestAgent, defect)

Description

[Julien Wajsberg [:julienw]]

STR:
* have the test agent server running in background
* open a big unit test file
* save the file twice: once first to trigger a run, and once while the first run is running

Expected:
* a second run runs after the running one

Actual:
* the first run is stopped, and then we must restart the test agent server to make it work again.

Comment 1

[James Lal [:lightsofapollo]]

Sounds like a test-agent bug? We probably don't (or can't) cancel one test run for another right now.

Comment 2

[Julien Wajsberg [:julienw]]

At one point, I think the requests were queued... So I think this is a regression.

Comment 3

[Julien Wajsberg [:julienw]]

I have a WIP in https://github.com/julienw/js-test-agent/compare/892048-fix-race-between-tests

We don't get the "test runner end" event when running "APP=<app> make test-agent-test", I don't know the reason yet.

James, do you think the queuing logic should go to the browser-worker instead of the queue-tests file?

Comment 4

[James Lal [:lightsofapollo]]

hrm... Its been awhile since I looked at this code but I think it needs to have some relationship with the server side since it manages when is "done" by keeping track of the completed events sent by the worker.

Comment 5

[Julien Wajsberg [:julienw]]

AFAIK the server-side does not manage at all when is "done" currently.

I've finally resolved the issue in comment 3 but I've been thinking more about this and I think I'll redo it in the browser.

Comment 6

[Julien Wajsberg [:julienw]]

Attachment: github PR

I'm not requesting review yet because I want to properly test the latest code.

Comment 7

[Julien Wajsberg [:julienw]]

Comment on attachment 8375450 [details] [review]
github PR

ok, this works fine for me.

I have one caveat with the following STR:
* save a file that is watched by the process
* run "make test-agent-test" in a separate terminal
=> the output of the watched file's run is terminated because of the capture.

There are several solutions to this:
* the simplest solution is to simply stop the capture, but I think it would be ugly in travis
* a more elaborate solution is to track when a test starts/ends, I started doing this but this is not _that_ easy. I think the ultimate solution would be to have the same id for mirroring and for the request, so that we can couple these.

But I think this is for another bug since this patch fixes the main most important issue.

Comment 8

[Julien Wajsberg [:julienw]]

Comment on attachment 8375450 [details] [review]
github PR

Yuren, if you can review this, since James seems to be away :)

Comment 9

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8375450 [details] [review]
github PR

also set feedback flag to ricky to get more feedback.

Comment 10

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8375450 [details] [review]
github PR

looks good to me if nits are addressed, although I have a question for _sendBatchRequest() but I bet you have a good reason for it, so r=yurenju :-)

Comment 11

[Julien Wajsberg [:julienw]]

master: a2f0b1a797a620ddab5d7ee4dd54090fc01a2254