Skip to content
This repository has been archived by the owner on Mar 8, 2023. It is now read-only.

HARP 14470 #2173

Merged
merged 14 commits into from
May 4, 2021
Merged

HARP 14470 #2173

merged 14 commits into from
May 4, 2021

Conversation

nzjony
Copy link
Member

@nzjony nzjony commented Apr 19, 2021

  • nyc only supports code coverage in node, however we have a number of browser based tests, and it is the primary use case, so we have decided to do the code coverage in karma.
  • the karma.conf.js previously used the pre-bundled test bundle, but this doesn't give us the code coverage per line number, I could have tried to find a way to remap the bundle to lines, but because the karma-typescript project is well maintained, I went this road. This however means there is a lot of configuration to get the tests running correctly, especially those that need to load resources.
  • note also, there are quite a few tests that time out when run in the browser, hence the need to change the timeout in some cases
  • note also, karma outputs the code coverage format using lcov, this is different to nyc and means that the code coverage drops a lot, this is because the lcov format can tell if a line is fully covered, i.e. for an if statement, it records if both true and false variants are executed. As such, lines without full branch coverage are treated as not covered, and this reduces the code coverage in the codecov.io report available via github. The local text report which is shown when running yarn cov-test doesn't have this behaviour, i.e lines that have just one branch executed are counted as part of the code coverage.
  • moved most of the karma.conf.js to karma.options.js, so it can be shared and extended with the sdk and added the ability to prefix the files / exclude so that this repo works next to the sdk correctly.

@nzjony nzjony force-pushed the HARP-14470 branch 7 times, most recently from fe8fe59 to b51795e Compare April 22, 2021 07:46
@nzjony nzjony marked this pull request as ready for review April 22, 2021 10:21
- nyc only supports code coverage in node, however
  we have a number of browser based tests, and it
  is the primary use case, so we have decided to
  do the code coverage in karma.
- the karma.conf.js previously used the
  pre-bundled test bundle, but this doesn't give
  us the code coverage per line number, I could
  have tried to find a way to remap the bundle to
  lines, but because the karma-typescript project
  is well maintained, I went this road. This
  however means there is a lot of configuration to
  get the tests running correctly, especially
  those that need to load resources.
- note also, there are quite a few tests that time
  out when run in the browser, hence the need to
  change the timeout in some cases
- note also, karma outputs the code coverage
  format using lcov, this is different to nyc and
  means that the code coverage drops a lot, this
  is because the lcov format can tell if a line is
  fully covered, i.e. for an if statement, it
  records if both true and false variants are
  executed. As such, lines without full branch
  coverage are treated as not covered, and this
  reduces the code coverage in the codecov.io
  report available via github. The local text
  report which is shown when running `yarn
  cov-test` doesn't have this behaviour, i.e lines
  that have just one branch executed are counted
  as part of the code coverage.

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>
Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>
Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>
…nd extend the configuration

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>
Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Apr 28, 2021

Codecov Report

Merging #2173 (e5e932e) into master (b5c7a1a) will decrease coverage by 1.79%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2173      +/-   ##
==========================================
- Coverage   68.44%   66.64%   -1.80%     
==========================================
  Files         299      312      +13     
  Lines       26526    27745    +1219     
  Branches     6009     6199     +190     
==========================================
+ Hits        18156    18491     +335     
- Misses       8370     9254     +884     
Impacted Files Coverage Δ
@here/harp-test-utils/lib/ProfileHelper.ts 17.44% <ø> (ø)
@here/harp-utils/lib/UrlPlatformUtils.ts 80.00% <75.00%> (-20.00%) ⬇️
@here/harp-utils/lib/PerformanceTimer.ts 80.00% <0.00%> (-10.00%) ⬇️
@here/harp-mapview/lib/CameraMovementDetector.ts 87.32% <0.00%> (-5.64%) ⬇️
@here/harp-mapview/lib/MapView.ts 73.32% <0.00%> (-0.34%) ⬇️
@here/harp-mapview/lib/ConcurrentWorkerSet.ts 66.66% <0.00%> (ø)
@here/harp-utils/lib/Logger/index.ts
...eojson-datasource/lib/GeoJsonTileDecoderService.ts
@here/harp-mapview/lib/composing/index.ts
...ere/harp-test-utils/lib/rendering/DomImageUtils.ts 12.35% <0.00%> (ø)
... and 28 more

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 b5c7a1a...e5e932e. Read the comment docs.

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>
@nzjony nzjony closed this Apr 29, 2021
@nzjony nzjony reopened this Apr 29, 2021
@nzjony
Copy link
Member Author

nzjony commented Apr 29, 2021

Closing / reopening to retrigger sdk pipeline.

… are better to compare

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>
@nzjony nzjony closed this Apr 29, 2021
@nzjony nzjony reopened this Apr 29, 2021
nzjony added 5 commits May 3, 2021 10:47
Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>
Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>
Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>
Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>
it is from MapBox, anything ending with node.ts or index*.ts, or anything
which is in a test/ directory

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>
Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>
@nzjony nzjony closed this May 3, 2021
@nzjony nzjony reopened this May 3, 2021
Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>
@nzjony nzjony closed this May 3, 2021
@nzjony nzjony reopened this May 3, 2021
@nzjony nzjony merged commit b63d78e into master May 4, 2021
@nzjony nzjony deleted the HARP-14470 branch May 4, 2021 12:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants