adobe / aio-cli-plugin-asset-compute

Asset Compute Plugin for Adobe I/O Command Line Interface
Apache License 2.0
1 stars 7 forks source link

Mock server test fixes #45

Closed tmathern closed 3 years ago

tmathern commented 3 years ago

Description

This is to have more useful logs when using mock-server. We have observed some flakyness recently, with mock-server failing to start.

The first thing here is to add more log so we can see more failure details. Second, given the logs, and the way the code is built, my hypothesis is that mock-server doesn't start fast enough. This results in an error being thrown and tests failing. We currently wait 10s for mock-server to start, I moved it up to 15s (maybe it will help ;) ).

Other things:

Fixes => no issue associated, just tests starting to fail everywhere...

Types of changes

Checklist:

codecov[bot] commented 3 years ago

Codecov Report

Merging #45 into master will decrease coverage by 0.31%. The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #45      +/-   ##
==========================================
- Coverage   90.62%   90.30%   -0.32%     
==========================================
  Files          13       13              
  Lines         821      825       +4     
==========================================
+ Hits          744      745       +1     
- Misses         77       80       +3     
Impacted Files Coverage Δ
src/lib/mockserver.js 88.05% <66.66%> (-4.01%) :arrow_down:
src/lib/testrunner.js 87.78% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7639af3...ac1b172. Read the comment docs.

tmathern commented 3 years ago

Note that I couldn't reproduce the flakyness locally either, which is a further argument of mock-server being slow to start (I think the macbook pro with 16Gb RAM and nothing else to do than run tests is a tad faster than a build server).

alexkli commented 3 years ago

@tmathern could you also make these changes:

We should probably catch the error differently, as right now it aborts all tests. But it would be good to track it as a test error (not failure). I think we should just move this line that starts the mocks a bit down into the try/catch block, which would handle that already.

And probably try/catch around _stopMocks() as well, with this._validateErrorResult(e) in the catch block.

tmathern commented 3 years ago

Just added the testrunner changes to the PR.

adobe-bot commented 3 years ago

:tada: This PR is included in version 1.4.5 :tada:

The release is available on:

Your semantic-release bot :package::rocket: