Skip to content

flow-coverage-report#11545

Merged
bvaughn merged 6 commits intofacebook:masterfrom
bvaughn:flow-coverage
Jan 9, 2018
Merged

flow-coverage-report#11545
bvaughn merged 6 commits intofacebook:masterfrom
bvaughn:flow-coverage

Conversation

@bvaughn
Copy link
Copy Markdown
Contributor

@bvaughn bvaughn commented Nov 13, 2017

During last weeks' sync in London, we chatted about Flow coverage stats and how we might track them over time. I noticed an NPM module called flow-coverage-report that seemed worth testing, so I thought I'd put up a PR for discussion.

Pros

  • The module generates a file in flow-coverage/flow-coverage.json that contains lots of useful stats that we could use to compare over time, including:
{
  threshold: 90,
  covered_count: 20770,
  uncovered_count: 10001,
  percent: 67,
  flowStatus: {
    flowVersion: "0.57.3",
    errors: [],
    passed: true
  },
  flowAnnotations: {
    passed: false,
    flowFiles: 72,
    flowWeakFiles: 0,
    noFlowFiles: 76,
    totalFiles: 148
  },
  files: {
    "path/to/file.js": {
      expressions: {
        covered_count: 31,
        uncovered_count: 38,
        uncovered_locs: [
          {
            source:
              "~/path/to/file.js",
            type: "SourceFile",
            start: { line: 10, column: 5, offset: 229 },
            end: { line: 10, column: 43, offset: 268 }
          },
          // ...
        ]
      },
      filename: "path/to/file.js",
      annotation: "no flow",
      percent: 44
    }
  },
  globIncludePatterns: ["packages/**/src/**/*.js"],
  globExcludePatterns: [
    "packages/**/__tests__/*.js",
    "packages/**/__mocks__/*.js"
  ],
}
  • CLI supports a "threshold" param, below which it will fail with a non-0 status, so it would be easy to fail on CI if coverage dropped below a certain amount. (I'm not sure we'd actually want to do this.)

Cons

  • This takes ~165 seconds to run (on my Macbook Pro).
  • I don't know how actively developed the tool is.

Example Syntax

yarn flow-coverage-report         \
  -i "packages/**/src/**/*.js"    \
  -x "packages/**/__tests__/*.js" \
  -x "packages/**/__mocks__/*.js" \
  -t html                         \
  -t json                         \
  -t text                         \
  --threshold 90

Output

ss-flow-coverage

@gaearon
Copy link
Copy Markdown
Collaborator

gaearon commented Jan 5, 2018

I'd be interested in getting it in if I can run it as yarn flow-coverage without all those arguments. I don't care about using it for CI or anything else, but I'd find it helpful for some local checks now and then.

@bvaughn
Copy link
Copy Markdown
Contributor Author

bvaughn commented Jan 5, 2018

Rebased and added a yarn flow-coverage command.

Copy link
Copy Markdown
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Comment thread .flow-coverage-config
@@ -0,0 +1,17 @@
{
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO would be nice to avoid another top level dotfile if we can.
I'd move it to scripts/flow/coverage-config.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how to prevent Flow from trying to parse the JSON file if I move it here. Even adding it to the [ignore] block in the .flowconfig isn't sufficient.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now I'm just going to leave this as a dotfile. I don't know how to make Flow ignore it (and not error) otherwise, and it doesn't seem pressing enough to invest more than a few minutes into.

@bvaughn bvaughn changed the title [Discussion only - not for merge] flow-coverage-report flow-coverage-report Jan 9, 2018
@bvaughn bvaughn merged commit 18288b2 into facebook:master Jan 9, 2018
@bvaughn bvaughn deleted the flow-coverage branch January 9, 2018 19:14
ManasJayanth pushed a commit to ManasJayanth/react that referenced this pull request Jan 12, 2018
* Added 'flow-coverage-report' package for discussion

* Aded flow-coverage command and configuration file

* Moved FLow coverage config file to scripts/flow/coverage-config

* Moved Flow coverage config back to root as dotfile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants