Coverage report #230

wzalazar · 2018-07-16T22:56:25Z

Description of Changes

Introduces report of coverage for unit test and integration test.

I run the coverage separated because are different instances and when you run another after the first one it will rewrite the input. Maybe when we have the all test in the same instance we will be able to capture the all the coverage together.

needed-by #11

PR Review Checklist

Have you tested the changes?
Is QE review required? If so, has this been reviewed by QE? (For all new features, review by QE is required. Minor changes which already have passing functional tests may not need a new QE review -- when in doubt, loop in a QE reviewer).
Are there code changes that require new unit tests? If so, are those tests included? (A separate PR for tests is not appropriate).
Are documentation updates required? If so, does this PR include it? (Separate documentation PR is generally not appropriate).
Are the added/edited files named appropriately and located correctly in the project directory structure?

lautarodragan · 2018-07-16T23:28:20Z

How does coverage output affect whether the Travis job fails or passes?

What's the scope of this PR?

Is there an associated GH issue?

wzalazar · 2018-07-16T23:31:54Z

the scope is only for showing the coverage report.

lautarodragan · 2018-07-17T00:50:23Z

Thanks.

I'm seeing the following output in Travis:

For unit tests (RITEway):

=============================== Coverage summary ===============================
Statements   : 13.33% ( 12/90 )
Branches     : 23.33% ( 7/30 )
Functions    : 10% ( 2/20 )
Lines        : 13.41% ( 11/82 )
================================================================================

For integration tests (Alsatian):

=============================== Coverage summary ===============================
Statements   : Unknown% ( 0/0 )
Branches     : Unknown% ( 0/0 )
Functions    : Unknown% ( 0/0 )
Lines        : Unknown% ( 0/0 )
================================================================================

Is this the expected output? What's up with the Unknown% ( 0/0 )?

lautarodragan · 2018-07-17T03:38:46Z

Also: it's a good opportunity to update the README's coverage section.

node/README.md

Lines 277 to 284 in 6a00eaf

    
           ### Coverage 
        
           Coverage is generated with [Istanbul](https://github.com/istanbuljs/nyc) whenever tests are run. 
        
           A more complete report can be generated by running `npm run coverage`. 
        
           This area needs some reviewing — the reports look a bit off sometimes. 
        
           Also, we'll want the Travis job to check that coverage doesn't go down with pull requests. See [#11][i11].

wzalazar · 2018-07-17T14:49:52Z

@lautarodragan
I will review the report of coverage of the integration test, but I'm thinking is because I added all files.

wzalazar · 2018-07-17T17:02:38Z

Regarding integration test:

well, before shown wrong information because it was over test files

-------------------------|----------|----------|----------|----------|-------------------|
File                     |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
-------------------------|----------|----------|----------|----------|-------------------|
All files                |    97.87 |      100 |       80 |    97.81 |                   |
 test                    |    66.67 |      100 |        0 |    66.67 |                   |
  Claims.ts              |     62.5 |      100 |        0 |     62.5 |          64,65,66 |
  Keys.ts                |      100 |      100 |      100 |      100 |                   |
 test/Integration        |      100 |      100 |      100 |      100 |                   |
  GetWork.ts             |      100 |      100 |      100 |      100 |                   |
  GetWorksByPublicKey.ts |      100 |      100 |      100 |      100 |                   |
  Helper.ts              |      100 |      100 |      100 |      100 |                   |
  PostWork.ts            |      100 |      100 |      100 |      100 |                   |
  SecurityHeaders.ts     |      100 |      100 |      100 |      100 |                   |
-------------------------|----------|----------|----------|----------|-------------------|

now the information it's about the app files

------------------------------|----------|----------|----------|----------|-------------------|
File                          |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
------------------------------|----------|----------|----------|----------|-------------------|
All files                     |        0 |        0 |        0 |        0 |                   |
 src                          |        0 |        0 |        0 |        0 |                   |
  Error.ts                    |        0 |        0 |      100 |        0 |             1,2,4 |
  index.ts                    |        0 |        0 |        0 |        0 |... 09,110,112,116 |
 src/API                      |        0 |      100 |      100 |        0 |                   |
  SecurityHeaders.ts          |        0 |      100 |      100 |        0 |                 1 |
 src/API/Middlewares          |        0 |        0 |        0 |        0 |                   |
  HttpExceptionsMiddleware.ts |        0 |        0 |        0 |        0 |... 14,16,17,19,20 |
  LoggerMiddleware.ts         |        0 |      100 |        0 |        0 |             4,8,9 |
 src/Extensions               |        0 |        0 |        0 |        0 |                   |
  Promise.ts                  |        0 |        0 |        0 |        0 |             5,8,9 |
 src/Helpers                  |        0 |        0 |        0 |        0 |                   |
  AsyncPipe.ts                |        0 |      100 |        0 |        0 |                 3 |
  Bitcoin.ts                  |        0 |        0 |        0 |        0 |... 11,13,25,29,30 |
  Logging.ts                  |        0 |      100 |        0 |        0 |        7,11,15,18 |
  Time.ts                     |        0 |      100 |        0 |        0 |               1,2 |
------------------------------|----------|----------|----------|----------|-------------------|
=============================== Coverage summary ===============================
Statements   : 0% ( 0/77 )
Branches     : 0% ( 0/22 )
Functions    : 0% ( 0/18 )
Lines        : 0% ( 0/70 )
================================================================================

wzalazar · 2018-07-17T18:17:55Z

well, we have a problem with the option --all of nyc. This option is omitting several files. I'm researching about it, I found a lot of solutions but none was fixed the problem.

lautarodragan · 2018-07-17T19:50:00Z

If it's only a problem with Alsatian maybe we can ignore it?

…-coverage

lautarodragan

Marking this one with Request changes for now

wzalazar · 2018-07-18T21:35:26Z

Report:

Unit tests: it's already. Several files are untracked until we will do the unit tests. It was tested.

Integration tests: At the moment there are several issues with this report. there is no official documentation about how to get the report for the integration tests.

I left this issue, is like an introduction to the problem
istanbuljs/nyc#548

@lautarodragan @warrenv

Keeping in mind that the report doesn't work properly. What is your opinion with the report for the integration test?

When we decided the path, I will update the README

warrenv · 2018-07-18T21:57:00Z

Is it possible report on untested files for the unit test? Or do they also have a problem with the --all flag?

wzalazar · 2018-07-18T22:01:43Z

@warrenv

Is it possible report on untested files for the unit test? Or do they also have a problem with the --all flag?

the --all option is in true. But all files that contain

interface
decorator

these will be tracked when you do the unit test.

Is a bug, I think yes

warrenv · 2018-07-19T01:51:10Z

ok. so typescript is interfering here somehow?
one of the purposes of running coverage reports is to expose files that don't have unit tests or are missing sufficient coverage.

wzalazar · 2018-07-19T17:40:53Z

seems is Typescript, I leave these links

Coverage reports don't include TS files unless explicitly compiled
istanbuljs/nyc#742
open: Dec 8 2017
last comment: Jun 15 2018
status: OPEN

--all does not include .ts files that use typecasting
istanbuljs/nyc#679
open: Sep 22 2017
last comment: Jun 16 2018
status: OPEN

Typescript code coverage with "all": true flag reports incorrect missing coverage
istanbuljs/nyc#643
open: Aug 11 2017
last comment: May 17 2018
status: CLOSED

Problems with --all
istanbuljs/nyc#603 (Jun 11 2017 - Mar 29 2018) OPEN
open: Jun 11 2017
last comment: Mar 29 2018
status: OPEN

--all fails with ts-node (Typescript)
istanbuljs/nyc#504
open: Jan 26 2017
last comment: 1 Jul 2018
status: CLOSED

Including all files when not using require or register
istanbuljs/nyc#776
open: Feb 10 2018
last comment: Feb 17 2018
status: OPEN

Better TypeScript coverage support in Istanbul
microsoft/TypeScript#24993
open: Jun 15 2018
last comment: Jun 15 2018
status: OPEN

warrenv · 2018-07-19T18:20:43Z

thanks for thoroughly documenting that Walter! If we made sure there was a unit test file to every code file (even if it only ran the most basic test), would that get us around the problem? Every file would be required so every file by definition would show up in the coverage report. We could write a script that makes sure every file has a matching unit test file and have travis run it so newly added files don't sneak thru.

…

On Thu, Jul 19, 2018 at 11:40 AM Walter Zalazar ***@***.***> wrote: seems is Typescript, I leave these links *Coverage reports don't include TS files unless explicitly compiled* istanbuljs/nyc#742 <istanbuljs/nyc#742> open: Dec 8 2017 last comment: Jun 15 2018 status: OPEN *--all does not include .ts files that use typecasting* istanbuljs/nyc#679 <istanbuljs/nyc#679> open: Sep 22 2017 last comment: Jun 16 2018 status: OPEN *Typescript code coverage with "all": true flag reports incorrect missing coverage* istanbuljs/nyc#643 <istanbuljs/nyc#643> open: Aug 11 2017 last comment: May 17 2018 status: CLOSED *Problems with --all* istanbuljs/nyc#603 <istanbuljs/nyc#603> (Jun 11 2017 - Mar 29 2018) OPEN open: Jun 11 2017 last comment: Mar 29 2018 status: OPEN *--all fails with ts-node (Typescript)* istanbuljs/nyc#504 <istanbuljs/nyc#504> open: Jan 26 2017 last comment: 1 Jul 2018 status: CLOSED *Including all files when not using require or register* istanbuljs/nyc#776 <istanbuljs/nyc#776> open: Feb 10 2018 last comment: Feb 17 2018 status: OPEN *Better TypeScript coverage support in Istanbul* microsoft/TypeScript#24993 <microsoft/TypeScript#24993> open: Jun 15 2018 last comment: Jun 15 2018 status: OPEN — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#230 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAB1z0SoO1bmo4pnexcAE_rPQLYoeCJ0ks5uIMSmgaJpZM4VR7Tl> .

wzalazar · 2018-07-23T23:35:54Z

our nyc is working now, this is the output the coverage of unit test:

------------------------------------------|----------|----------|----------|----------|-------------------|
File                                      |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
------------------------------------------|----------|----------|----------|----------|-------------------|
All files                                 |     0.74 |     3.45 |        0 |     0.79 |                   |
 src                                      |        0 |        0 |        0 |        0 |                   |
  Configuration.ts                        |        0 |        0 |        0 |        0 |... 06,107,108,116 |
  Error.ts                                |        0 |        0 |        0 |        0 |             1,2,4 |
  Interfaces.ts                           |        0 |        0 |        0 |        0 |                22 |
  index.ts                                |        0 |        0 |        0 |        0 |... 09,110,112,116 |
 src/API                                  |        0 |      100 |        0 |        0 |                   |
  API.ts                                  |        0 |      100 |        0 |        0 |... 45,46,47,48,49 |
  APIConfiguration.ts                     |        0 |        0 |        0 |        0 |                   |
  RouterConfiguration.ts                  |        0 |        0 |        0 |        0 |                   |
  SecurityHeaders.ts                      |        0 |      100 |      100 |        0 |                 1 |
 src/API/Middlewares                      |    10.53 |       25 |        0 |    11.11 |                   |
  HttpExceptionsMiddleware.ts             |        0 |        0 |        0 |        0 |... 14,16,17,19,20 |
  LoggerMiddleware.ts                     |        0 |      100 |        0 |        0 |             4,8,9 |
  RequestValidationMiddleware.ts          |      100 |      100 |      100 |      100 |                   |
 src/BlockchainReader                     |        0 |      100 |        0 |        0 |                   |
  BlockchainReader.ts                     |        0 |      100 |        0 |        0 |... 49,50,51,54,57 |
  BlockchainReaderConfiguration.ts        |        0 |        0 |        0 |        0 |                   |
  BlockchainReaderServiceConfiguration.ts |        0 |        0 |        0 |        0 |                   |
  ClaimControllerConfiguration.ts         |        0 |        0 |        0 |        0 |                   |
 src/BlockchainWriter                     |        0 |      100 |        0 |        0 |                   |
  BlockchainWriter.ts                     |        0 |      100 |        0 |        0 |... 61,64,65,71,72 |
  BlockchainWriterConfiguration.ts        |        0 |        0 |        0 |        0 |                   |
  ClaimControllerConfiguration.ts         |        0 |        0 |        0 |        0 |                   |
  ServiceConfiguration.ts                 |        0 |        0 |        0 |        0 |                   |
 src/Extensions                           |        0 |        0 |        0 |        0 |                   |
  Promise.ts                              |        0 |        0 |        0 |        0 |             5,8,9 |
 src/Helpers                              |        0 |        0 |        0 |        0 |                   |
  AsyncPipe.ts                            |        0 |      100 |        0 |        0 |                 3 |
  Bitcoin.ts                              |        0 |        0 |        0 |        0 |... 11,13,25,29,30 |
  FetchError.ts                           |        0 |        0 |        0 |        0 |          15,17,19 |
  Logging.ts                              |        0 |      100 |        0 |        0 |        7,11,15,18 |
  Time.ts                                 |        0 |      100 |        0 |        0 |               1,2 |
 src/Messaging                            |        0 |        0 |        0 |        0 |                   |
  Messages.ts                             |        0 |        0 |        0 |        0 |                   |
  Messaging.ts                            |        0 |        0 |        0 |        0 |... 03,106,108,111 |
 src/Storage                              |        0 |        0 |        0 |        0 |                   |
  ClaimControllerConfiguration.ts         |        0 |        0 |        0 |        0 |                   |
  DownloadFailure.ts                      |        0 |        0 |        0 |        0 |                   |
  Entry.ts                                |        0 |        0 |        0 |        0 |                   |
  Exceptions.ts                           |        0 |        0 |        0 |        0 |... 30,38,39,43,44 |
  IPFSConfiguration.ts                    |        0 |        0 |        0 |        0 |                   |
  ServiceConfiguration.ts                 |        0 |        0 |        0 |        0 |                   |
  Storage.ts                              |        0 |      100 |        0 |        0 |... 68,69,75,76,77 |
  StorageConfiguration.ts                 |        0 |        0 |        0 |        0 |                   |
 src/View                                 |        0 |      100 |        0 |        0 |                   |
  View.ts                                 |        0 |      100 |        0 |        0 |... 43,44,45,46,47 |
  ViewConfiguration.ts                    |        0 |        0 |        0 |        0 |                   |
------------------------------------------|----------|----------|----------|----------|-------------------|
=============================== Coverage summary ===============================
Statements   : 0.74% ( 2/269 )
Branches     : 3.45% ( 2/58 )
Functions    : 0% ( 0/70 )
Lines        : 0.79% ( 2/253 )
================================================================================

lautarodragan · 2018-07-24T01:16:58Z

package.json

@@ -68,10 +70,11 @@
    "babel-plugin-module-resolver": "3.1.1",
    "concurrently": "3.6.0",
    "nodemon": "1.18.3",
-    "nyc": "12.0.2",
+    "nyc": "https://github.com/poetapp/nyc.git",


Can you specify a commit hash? This way if changes are pushed the master branch of poetapp/nic they won't get picked up automatically by the node.

yes, good idea!!! 👍

lautarodragan · 2018-07-24T01:19:27Z

Looks like there's still some files missing from that report.

In src/API: Router.ts, WorkController.ts

wzalazar · 2018-07-24T14:51:49Z

Looks like there's still some files missing from that report.

In src/API: Router.ts, WorkController.ts

these files have a problem with the instrument because it has a decorator in the constructor. I have to create a unit test for this files.

…-coverage

warrenv

Great work @wzalazar!! This will really help us as we increase our unit test coverage.

wzalazar · 2018-07-26T15:31:56Z

@warrenv thanks!!!

from here we start...

=============================== Coverage summary ===============================
Statements   : 34.85% ( 237/680 )
Branches     : 4.35% ( 4/92 )
Functions    : 11.76% ( 14/119 )
Lines        : 33.87% ( 210/620 )
================================================================================

wzalazar · 2018-07-30T16:48:44Z

@lautarodragan @warrenv

I need your review

lautarodragan

Excellent work @wzalazar !

coverage report

2a36e86

wzalazar requested review from lautarodragan and a team as code owners July 16, 2018 22:56

wzalazar added the quality label Jul 16, 2018

refactor tasks

94230eb

wzalazar added the don't merge label Jul 16, 2018

fix report integration

ace40f3

warrenv assigned warrenv and wzalazar and unassigned warrenv Jul 17, 2018

Merge branch 'master' into report-coverage

4b2e7c8

lautarodragan and others added 3 commits July 17, 2018 19:46

Merge branch 'master' into report-coverage

f094e7a

fix, requires

edd46e6

Merge branch 'report-coverage' of github.com:poetapp/node into report…

6d74bc7

…-coverage

lautarodragan requested changes Jul 18, 2018

View reviewed changes

Merge branch 'master' into report-coverage

ae2e831

lautarodragan reviewed Jul 24, 2018

View reviewed changes

Merge branch 'master' into report-coverage

853da3b

wzalazar mentioned this pull request Jul 24, 2018

Nyc dependency #252

Closed

wzalazar added 2 commits July 24, 2018 13:16

shorthand dependency

4cb326a

Merge branch 'report-coverage' of github.com:poetapp/node into report…

ef5ad3d

…-coverage

wzalazar mentioned this pull request Jul 24, 2018

Create the unit test for files untracked of report coverage #253

Closed

13 tasks

wzalazar added 2 commits July 24, 2018 18:04

unit test

b5dc0d9

fix lint

43891cd

wzalazar mentioned this pull request Jul 25, 2018

Documentation coverage #258

Closed

wzalazar added 4 commits July 25, 2018 13:22

update docs

432b797

fix docs

2f66f80

fix docs

e65c855

fix docs

0efa453

warrenv previously approved these changes Jul 26, 2018

View reviewed changes

improve documentation

61468be

wzalazar dismissed warrenv’s stale review via 61468be July 26, 2018 15:22

wzalazar removed the don't merge label Jul 26, 2018

Merge branch 'master' into report-coverage

3914305

warrenv approved these changes Jul 30, 2018

View reviewed changes

lautarodragan approved these changes Jul 30, 2018

View reviewed changes

lautarodragan mentioned this pull request Jul 30, 2018

Mocked unit test example #273

Closed

wzalazar merged commit 03e479b into master Jul 30, 2018

wzalazar deleted the report-coverage branch July 30, 2018 21:46

kennylavender mentioned this pull request Aug 27, 2018

Extract StorageWriter from Storage #364

Merged

wzalazar mentioned this pull request Dec 7, 2018

feat: Option Plugins istanbuljs/nyc#948

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Coverage report #230

Coverage report #230

wzalazar commented Jul 16, 2018 •

edited by lautarodragan

Loading

lautarodragan commented Jul 16, 2018

wzalazar commented Jul 16, 2018

lautarodragan commented Jul 17, 2018

lautarodragan commented Jul 17, 2018

wzalazar commented Jul 17, 2018

wzalazar commented Jul 17, 2018

wzalazar commented Jul 17, 2018

lautarodragan commented Jul 17, 2018

lautarodragan left a comment

wzalazar commented Jul 18, 2018 •

edited

Loading

warrenv commented Jul 18, 2018

wzalazar commented Jul 18, 2018 •

edited

Loading

warrenv commented Jul 19, 2018

wzalazar commented Jul 19, 2018

warrenv commented Jul 19, 2018 via email

wzalazar commented Jul 23, 2018

lautarodragan Jul 24, 2018

wzalazar Jul 24, 2018

lautarodragan commented Jul 24, 2018

wzalazar commented Jul 24, 2018

warrenv left a comment

wzalazar commented Jul 26, 2018

wzalazar commented Jul 30, 2018

lautarodragan left a comment

Coverage report #230

Coverage report #230

Conversation

wzalazar commented Jul 16, 2018 • edited by lautarodragan Loading

Description of Changes

PR Review Checklist

lautarodragan commented Jul 16, 2018

wzalazar commented Jul 16, 2018

lautarodragan commented Jul 17, 2018

lautarodragan commented Jul 17, 2018

wzalazar commented Jul 17, 2018

wzalazar commented Jul 17, 2018

wzalazar commented Jul 17, 2018

lautarodragan commented Jul 17, 2018

lautarodragan left a comment

Choose a reason for hiding this comment

wzalazar commented Jul 18, 2018 • edited Loading

warrenv commented Jul 18, 2018

wzalazar commented Jul 18, 2018 • edited Loading

warrenv commented Jul 19, 2018

wzalazar commented Jul 19, 2018

warrenv commented Jul 19, 2018 via email

wzalazar commented Jul 23, 2018

lautarodragan Jul 24, 2018

Choose a reason for hiding this comment

wzalazar Jul 24, 2018

Choose a reason for hiding this comment

lautarodragan commented Jul 24, 2018

wzalazar commented Jul 24, 2018

warrenv left a comment

Choose a reason for hiding this comment

wzalazar commented Jul 26, 2018

wzalazar commented Jul 30, 2018

lautarodragan left a comment

Choose a reason for hiding this comment

wzalazar commented Jul 16, 2018 •

edited by lautarodragan

Loading

wzalazar commented Jul 18, 2018 •

edited

Loading

wzalazar commented Jul 18, 2018 •

edited

Loading