Skip to content

Conversation

@elas7
Copy link
Contributor

@elas7 elas7 commented Aug 14, 2018

flow-coverage-report stopped working after Flow was set to run for each renderer separately (#12846). It happened because flow-coverage-report calls flow coverage, and that command doesn't work without a .flowconfig in a parent directory of every file checked.

This is fixed by creating a temporary .flowconfig in the project root before running flow-coverage-report.

This fix also allows to move .flowcoverage out of the project root, and to select a renderer before generating the coverage report.

Internally, this is done by rewriting writeConfig() to accept a rootFolder argument. If rootFolder is true, a config is made in the root folder, with the right relative paths.

Before
before

After
after

`flow-coverage-report` stopped working after Flow was set to run for each renderer separately (facebook#12846). It happened because `flow-coverage-report` calls `flow coverage`, and that command doesn't work without a `.flowconfig` in a parent directory of every file checked.

This is fixed by creating a temporary `.flowconfig` in the project root before running `flow-coverage-report`.

This fix also allows to move `.flowcoverage` out of the project root, and to select a renderer before generating the coverage report.

Internally, this is done by rewriting `writeConfig()` to accept a `rootFolder` argument. If `rootFolder` is true, a config is made in the root folder, with the right relative paths.
@gaearon
Copy link
Collaborator

gaearon commented Aug 14, 2018

Thanks for the PR! The libs substitution is more complex than I would like.. Ideally we want to avoid as much abstraction as we can there.

Maybe let's just remove this script? We don't really use it. Or find some other solution that doesn't involve making the config more abstract.

@elas7
Copy link
Contributor Author

elas7 commented Aug 14, 2018

Yes, makes sense.

I don't think there's a way to solve this without adding complexity to .flowconfig's generation. I'll make a PR to remove the script.

elas7 added a commit to elas7/react that referenced this pull request Aug 14, 2018
`flow-coverage-report` stopped working after Flow was set to run for each renderer separately (facebook#12846). As discussed in facebook#13393, this is hard to fix without adding complexity to `.flowconfig`'s generation.
@elas7 elas7 closed this Aug 14, 2018
gaearon pushed a commit that referenced this pull request Aug 14, 2018
`flow-coverage-report` stopped working after Flow was set to run for each renderer separately (#12846). As discussed in #13393, this is hard to fix without adding complexity to `.flowconfig`'s generation.
@elas7 elas7 deleted the fix-flow-coverage branch August 14, 2018 20:20
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