Wait 10 seconds to make sure browser instances are closed

[luixxiul]

There are several cases where a new browser instance starts with .startApp() before the existing instance is completely closed with .stopApp(). Adding 10 seconds delay should almost assure that the existing instance is closed before new one starts on a slow machine.

Closes #10490

Auditors:

Test Plan:

1. Run npm run test -- --grep='sessionStore test'

2. Run npm run test -- --grep='ledger history'

3. Check the Travis result as well

   https://travis-ci.org/brave/browser-laptop/jobs/286708254#L3528

Submitter Checklist:

• Submitted a ticket for my issue if one did not already exist.
• Used Github auto-closing keywords in the commit message.
• Added/updated tests for this change (for new code or code which already has tests).
• Ran git rebase -i to squash commits (if needed).
• Tagged reviewers and labelled the pull request as needed.

Test Plan:

Reviewer Checklist:

Tests

• Adequate test coverage exists to prevent regressions
• Tests should be independent and work correctly when run individually or as a suite ref
• New files have MPL2 license header

[luixxiul]

The change looks working properly:

https://travis-ci.org/brave/browser-laptop/jobs/286708254#L3528

 23 passing (4m)

[codecov-io]

Codecov Report

Merging #11239 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #11239      +/-   ##
==========================================
- Coverage   52.09%   52.06%   -0.04%     
==========================================
  Files         269      269              
  Lines       25501    25501              
  Branches     4062     4062              
==========================================
- Hits        13286    13277       -9     
- Misses      12215    12224       +9

Flag	Coverage Δ
#unittest	52.06% <ø> (-0.04%)	⬇️

Impacted Files	Coverage Δ
js/stores/appStoreRenderer.js	91.17% <0%> (-8.83%)	⬇️
app/renderer/components/reduxComponent.js	84.37% <0%> (-6.25%)	⬇️
js/stores/windowStore.js	27.27% <0%> (-0.31%)	⬇️

[luixxiul]

+1 from #11508

[cezaraugusto]

Looks good to me but I'm not sure if 10s is an OK number so deferring merge to @bsclifton @bbondy

[cezaraugusto]

also pls put ledger tests in test plan too as their files were changed too and rebase against master latest as new tests were added. Thanks

[luixxiul]

Thanks, the test was added to the 1st post.

[luixxiul]

Is this fix not needed? I am not quite sure about that.

[bbondy]

We're just not tracking things in milestones anymore unless it blocks the release, see #general in slack. Things can get uplifted from backlog as they are done mostly.

[luixxiul]

@bbondy I have seen that, and I am just wondering if the review on #11239 (review) above is not sufficient.

[bbondy]

Do you know how long it adds to the test suite total?

[luixxiul]

120 seconds are added. As the test runs parallel on Travis, it practically adds 80 seconds (sessionStoreTest).

[bbondy]

master: 41a0523
0.21.x: cd94bd9
0.20.x: c3c17d9
0.19.x: 3e6cdc5

[bsclifton]

@luixxiul the tests aren't parallelized like that, unfortunately. There are 8 simple groupings that will run at the same time... The changes here affect 3 groupings. So each of those 3 groups would be affected and delayed, because each group runs in a serial fashion

@luixxiul let's keep an eye on tests if you don't mind and ensure this doesn't eat up too much time

[bsclifton]

++ notes left:
#11239 (comment)