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

new: support for showing coverage on single function test. closes #1637 #1638

Merged
merged 4 commits into from
May 2, 2018
Merged

new: support for showing coverage on single function test. closes #1637 #1638

merged 4 commits into from
May 2, 2018

Conversation

primalmotion
Copy link
Contributor

I took a shot at implementing #1637.

@msftclas
Copy link

msftclas commented Apr 20, 2018

CLA assistant check
All CLA requirements met.

@primalmotion
Copy link
Contributor Author

hum the test failed, on go tip, and I don't think really understand how it would be related to my changes :/

@lggomez
Copy link
Contributor

lggomez commented Apr 27, 2018

@primalmotion the ci log shows that the linter failed: https://travis-ci.org/Microsoft/vscode-go/jobs/368922868#L877

Run the TS linter on the files you changed and try again

You can install it here: https://marketplace.visualstudio.com/items?itemName=eg2.tslint

@primalmotion
Copy link
Contributor Author

Subtle :) I’ll fix it asap

@primalmotion
Copy link
Contributor Author

primalmotion commented Apr 28, 2018

lint is fixed, but tests are failing on go:tip

Copy link
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

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

I am wary on re-using the setting coverOnTestPackage for this purpose.
coverOnTestPackage setting is true by default. So when people run all tests in a package, all related code files get coverage. Which makes sense, because all applicable tests are run and the coverage is shown.

If we re-use this setting, then when users run individual tests, then the coverage will show up by default, probably highlighting most of the code in red (assuming the single test was covering a very small portion of the complete code)

Also, the same wont happen when the user runs all tests in current file. So there is an inconsistency.

I would suggest to add a new setting instead coverOnSingleTest and keep it false by default.

@primalmotion
Copy link
Contributor Author

Sure I can add a new parameter.

@primalmotion
Copy link
Contributor Author

@ramya-rao-a added requested changes

@ramya-rao-a ramya-rao-a merged commit d464a17 into microsoft:master May 2, 2018
@ramya-rao-a
Copy link
Contributor

Thanks!

@primalmotion primalmotion deleted the cover-on-single-function branch May 2, 2018 21:29
@primalmotion
Copy link
Contributor Author

Thanks!

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.

4 participants