Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding test coverage with nyc #622

Closed
wants to merge 4 commits into from
Closed

adding test coverage with nyc #622

wants to merge 4 commits into from

Conversation

benhalverson
Copy link
Contributor

I tested this on both the dev and dev-3.0 branch. I can make the pull request to either branch.
@bmuenzenmeyer @raphaelokon
Summary of changes:

  • Adds test coverage using the nyc package
  • updated .gitignore to exclude ..nyc_output/
  • updated npm test command to include nyc command

@bmuenzenmeyer
Copy link
Member

I love the idea of understanding our code coverage and endeavoring to improve it.

In your tests, what was the output coverage? 😈

I want to review what tap already comes with to understand if the additional, direct dev dependency is needed. http://www.node-tap.org/coverage/

@raphaelokon
Copy link
Contributor

I just used node-tap without any deps installed … gave me:

------------------------|----------|----------|----------|----------|----------------|
File                    |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
------------------------|----------|----------|----------|----------|----------------|
All files               |    77.69 |       50 |    64.29 |    78.76 |                |
 bin                    |    77.98 |    55.88 |    71.15 |     79.6 |                |
  archive.js            |    88.24 |       50 |    66.67 |    93.75 |             25 |
  ask.js                |    53.33 |        0 |        0 |    61.54 | 20,25,30,33,35 |
  build.js              |    85.71 |    83.33 |    66.67 |    92.31 |             26 |
  check-args.js         |       20 |        0 |        0 |       20 |... 24,25,26,28 |
  check-overwrites.js   |       50 |        0 |        0 |       50 |    14,15,17,19 |
  copy-source-files.js  |      100 |      100 |      100 |      100 |                |
  default-config.js     |      100 |      100 |      100 |      100 |                |
  install-edition.js    |      100 |       50 |      100 |      100 |                |
  install-plugin.js     |    91.67 |       75 |      100 |    90.91 |             12 |
  install-starterkit.js |    93.33 |       75 |      100 |    92.86 |             15 |
  patternlab.js         |    96.15 |      100 |       80 |      100 |                |
  preview.js            |    34.15 |    58.33 |    14.29 |    37.84 |... 105,109,111 |
  replace-config.js     |      100 |      100 |      100 |      100 |                |
  resolve-config.js     |    84.21 |      100 |      100 |    84.21 |       34,35,36 |
  scaffold.js           |      100 |      100 |      100 |      100 |                |
  utils.js              |    92.86 |       70 |    92.86 |    93.75 |     21,112,113 |
  validate-config.js    |       50 |        0 |        0 |       50 |              9 |
 bin/cli-actions        |    77.11 |    33.33 |    53.33 |    76.62 |                |
  build.js              |      100 |      100 |      100 |      100 |                |
  export.js             |      100 |      100 |      100 |      100 |                |
  help.js               |    14.29 |      100 |        0 |    14.29 |    4,5,6,7,8,9 |
  init.js               |    96.88 |       50 |      100 |    96.67 |             44 |
  install.js            |    38.89 |        0 |        0 |    41.18 |... 21,22,23,30 |
  serve.js              |     87.5 |      100 |      100 |    85.71 |              8 |
 bin/inquiries          |       75 |      100 |        0 |       75 |                |
  confirm.js            |      100 |      100 |      100 |      100 |                |
  edition.js            |    57.14 |      100 |        0 |    57.14 |       11,12,35 |
  starterkit.js         |      100 |      100 |      100 |      100 |                |
------------------------|----------|----------|----------|----------|----------------|

@benhalverson
Copy link
Contributor Author

@bmuenzenmeyer Here's the test coverage report when I do nyc npm test on the dev branch

401 passing (9s)
--------------------------|----------|----------|----------|----------|----------------|
File                      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
--------------------------|----------|----------|----------|----------|----------------|
All files                 |    63.83 |    61.03 |    70.85 |    64.31 |                |
 annotation_exporter.js   |    91.84 |       75 |      100 |    91.84 |    23,35,36,37 |
 changes_hunter.js        |    85.19 |    56.25 |      100 |    85.19 |    48,50,51,65 |
 lineage_hunter.js        |    96.36 |    79.41 |      100 |    96.36 |        123,131 |
 list_item_hunter.js      |     87.5 |    85.71 |      100 |     87.5 |... 68,69,92,93 |
 markdown_parser.js       |    86.36 |     87.5 |      100 |    86.36 |       40,41,42 |
 object_factory.js        |    95.65 |    86.67 |      100 |    95.65 |    103,108,125 |
 parameter_hunter.js      |    95.19 |    91.67 |      100 |    95.19 |... 165,166,249 |
 pattern_assembler.js     |    71.54 |    66.67 |    81.08 |    71.37 |... 581,584,599 |
 pattern_engines.js       |    96.77 |    92.86 |      100 |    98.33 |             85 |
 pattern_graph.js         |    87.18 |    83.33 |    84.21 |    88.18 |... 411,412,413 |
 pattern_graph_dot.js     |        8 |        0 |        0 |     8.16 |... 139,140,141 |
 pattern_registry.js      |    93.75 |     62.5 |      100 |    93.75 |          66,82 |
 patternlab.js            |    12.17 |     2.67 |    14.63 |    12.67 |... 625,628,631 |
 plugin_manager.js        |     4.88 |        0 |        0 |     4.88 |... 115,118,121 |
 pseudopattern_hunter.js  |    92.11 |       75 |      100 |    92.11 |       34,42,43 |
 style_modifier_hunter.js |    90.91 |    83.33 |      100 |    90.91 |             21 |
 ui_builder.js            |    67.25 |    63.48 |    90.63 |    67.54 |... 663,666,671 |
 utilities.js             |     58.7 |       40 |       50 |    57.78 |... 135,138,139 |
--------------------------|----------|----------|----------|----------|----------------|
--------------------------|----------|----------|----------|----------|----------------|
File                      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
--------------------------|----------|----------|----------|----------|----------------|
All files                 |    63.83 |    61.03 |    70.85 |    64.31 |                |
 annotation_exporter.js   |    91.84 |       75 |      100 |    91.84 |    23,35,36,37 |
 changes_hunter.js        |    85.19 |    56.25 |      100 |    85.19 |    48,50,51,65 |
 lineage_hunter.js        |    96.36 |    79.41 |      100 |    96.36 |        123,131 |
 list_item_hunter.js      |     87.5 |    85.71 |      100 |     87.5 |... 68,69,92,93 |
 markdown_parser.js       |    86.36 |     87.5 |      100 |    86.36 |       40,41,42 |
 object_factory.js        |    95.65 |    86.67 |      100 |    95.65 |    103,108,125 |
 parameter_hunter.js      |    95.19 |    91.67 |      100 |    95.19 |... 165,166,249 |
 pattern_assembler.js     |    71.54 |    66.67 |    81.08 |    71.37 |... 581,584,599 |
 pattern_engines.js       |    96.77 |    92.86 |      100 |    98.33 |             85 |
 pattern_graph.js         |    87.18 |    83.33 |    84.21 |    88.18 |... 411,412,413 |
 pattern_graph_dot.js     |        8 |        0 |        0 |     8.16 |... 139,140,141 |
 pattern_registry.js      |    93.75 |     62.5 |      100 |    93.75 |          66,82 |
 patternlab.js            |    12.17 |     2.67 |    14.63 |    12.67 |... 625,628,631 |
 plugin_manager.js        |     4.88 |        0 |        0 |     4.88 |... 115,118,121 |
 pseudopattern_hunter.js  |    92.11 |       75 |      100 |    92.11 |       34,42,43 |
 style_modifier_hunter.js |    90.91 |    83.33 |      100 |    90.91 |             21 |
 ui_builder.js            |    67.25 |    63.48 |    90.63 |    67.54 |... 663,666,671 |
 utilities.js             |     58.7 |       40 |       50 |    57.78 |... 135,138,139 |
--------------------------|----------|----------|----------|----------|----------------|

On the 3.0 branch 1 test is failing. same command nyc npm test

  1 failing

  1) test/lineage_hunter_tests.js cascade_pattern_states sets the pattern state on any lineage patterns reverse lineage should be equal:

      Error: should be equal
      + expected - actual

      -complete
      +inreview

      at test/lineage_hunter_tests.js:289:8
      at Object.<anonymous> (test/lineage_hunter_tests.js:265:5)
      at run (bootstrap_node.js:420:7)
      at startup (bootstrap_node.js:139:9)
      at bootstrap_node.js:535:3

npm ERR! Test failed.  See above for more details.
--------------------------|----------|----------|----------|----------|----------------|
File                      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
--------------------------|----------|----------|----------|----------|----------------|
All files                 |    63.95 |    61.67 |    69.92 |    64.48 |                |
 annotation_exporter.js   |    91.67 |       75 |      100 |    91.67 |    23,35,36,37 |
 changes_hunter.js        |    88.89 |     62.5 |      100 |    88.89 |       48,50,51 |
 lineage_hunter.js        |    96.36 |    85.29 |      100 |    96.36 |        123,131 |
 list_item_hunter.js      |     87.5 |    85.71 |      100 |     87.5 |... 67,68,91,92 |
 markdown_parser.js       |    86.36 |     87.5 |      100 |    86.36 |       39,40,41 |
 object_factory.js        |    95.65 |    86.67 |      100 |    95.65 |    110,115,132 |
 parameter_hunter.js      |    95.19 |    91.67 |      100 |    95.19 |... 164,165,248 |
 pattern_assembler.js     |    71.48 |    66.67 |     82.5 |     71.7 |... 595,598,616 |
 pattern_engines.js       |    96.67 |    93.33 |      100 |    98.28 |             98 |
 pattern_graph.js         |    87.18 |    83.33 |    84.21 |    88.18 |... 411,412,413 |
 pattern_graph_dot.js     |        8 |        0 |        0 |     8.16 |... 139,140,141 |
 pattern_registry.js      |    93.75 |     62.5 |      100 |    93.75 |          66,82 |
 patternlab.js            |    13.38 |     2.67 |    18.75 |    13.91 |... 668,671,674 |
 plugin_manager.js        |     4.88 |        0 |        0 |     4.88 |... 115,118,121 |
 pseudopattern_hunter.js  |    90.74 |       75 |      100 |    90.74 | 33,41,42,87,88 |
 style_modifier_hunter.js |    90.91 |    83.33 |      100 |    90.91 |             21 |
 ui_builder.js            |    67.25 |    63.48 |    90.63 |    67.54 |... 665,668,673 |
 utilities.js             |       58 |       40 |       50 |    57.14 |... 139,150,151 |
--------------------------|----------|----------|----------|----------|----------------|```

@raphaelokon
Copy link
Contributor

raphaelokon commented Feb 14, 2017

@benhalverson Is there a reason why we have nyc as separate dependency? node-tap already have it built-in. So this
https://github.com/pattern-lab/patternlab-node/blob/dev-3.0/package.json#L56-L59
should probably just be "test": "eslint core/**/*.js && tap test/*_tests.js --reporter spec --coverage"

On that note …

  "scripts": {
    "test": "eslint core/**/*.js && tap test/*_tests.js --reporter spec",
    "lint": "eslint core/**/*.js"
  },

=>

  "scripts": {
    "test": "tap test/*_tests.js --reporter spec --coverage",
    "pretest": "npm run lint", 
    "lint": "eslint core/**/*.js"
  },

@bmuenzenmeyer bmuenzenmeyer self-assigned this Mar 20, 2017
bmuenzenmeyer added a commit that referenced this pull request Mar 21, 2017
part of resolving #622
bmuenzenmeyer added a commit that referenced this pull request Mar 21, 2017
added coverage to test command
part of resolving #622
@bmuenzenmeyer
Copy link
Member

I am going to close this as unneeded, considering the built in power tap has.
I love the idea of reporting code coverage for the library, and as you can see it's a mixed bag, with plenty to improve

as always, thanks for the thought and work you put into this Ben

@bmuenzenmeyer
Copy link
Member

The changelog will reflect this conversation and credit you both for the discussion and work you put into this.

Thanks

https://github.com/pattern-lab/patternlab-node/wiki/ChangeLog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants