Skip to content

Make proper unit tests for EnvironmentVariablesService. - #9704

Merged
Eric Snow (ericsnowcurrently) merged 2 commits into
microsoft:masterfrom
ericsnowcurrently:env-vars-functional-tests
Jan 22, 2020
Merged

Make proper unit tests for EnvironmentVariablesService.#9704
Eric Snow (ericsnowcurrently) merged 2 commits into
microsoft:masterfrom
ericsnowcurrently:env-vars-functional-tests

Conversation

@ericsnowcurrently

@ericsnowcurrently Eric Snow (ericsnowcurrently) commented Jan 21, 2020

Copy link
Copy Markdown

(for #8895)

This change helps simplify the changes in #9499.

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
  • Title summarizes what is changing
  • [ ] Has a news entry file (remember to thank yourself!)
  • [ ] Appropriate comments and documentation strings in the code
  • [ ] Has sufficient logging.
  • [ ] Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated
  • [ ] Test plan is updated as appropriate
  • [ ] package-lock.json has been regenerated by running npm install (if dependencies have changed)
  • [ ] The wiki is updated with any design decisions/details.

@ericsnowcurrently Eric Snow (ericsnowcurrently) added the no-changelog No news entry required label Jan 21, 2020
@codecov-io

Codecov (codecov-io) commented Jan 21, 2020 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #9704 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9704      +/-   ##
==========================================
- Coverage   60.79%   60.78%   -0.02%     
==========================================
  Files         551      551              
  Lines       29668    29795     +127     
  Branches     4485     4506      +21     
==========================================
+ Hits        18038    18110      +72     
- Misses      10621    10672      +51     
- Partials     1009     1013       +4
Impacted Files Coverage Δ
src/client/common/variables/environment.ts 92.68% <100%> (+0.47%) ⬆️
src/client/common/platform/pathUtils.ts 52.38% <0%> (-9.53%) ⬇️
src/client/common/platform/fs-paths.ts 68.36% <0%> (-3.57%) ⬇️
src/client/common/platform/fileSystem.ts 25.85% <0%> (-0.9%) ⬇️
src/client/common/platform/types.ts 100% <0%> (ø) ⬆️
...terpreter/locators/services/condaEnvFileService.ts 90.24% <0%> (+3.96%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c370167...986c9cf. Read the comment docs.

karrtikr
Kartik Raj (karrtikr) suggested changes Jan 21, 2020
View reviewed changes

@karrtikr Kartik Raj (karrtikr) left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please manually run CI against it. Functional tests are not run in PR validation pipeline.

Comment thread src/client/common/variables/environment.ts Outdated
@sonarqubecloud

sonarqubecloud Bot commented Jan 22, 2020

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ericsnowcurrently

Eric Snow (ericsnowcurrently) commented Jan 22, 2020 •
edited
Loading

Copy link
Copy Markdown
Author

I'm running the full CI tests, but running them on every PR is not going to be sustainable long-term. They simply take too long. 😞 We need to come up with a sensible criteria for when they should be run manually prior to merge.

@karrtikr

Kartik Raj (karrtikr) commented Jan 22, 2020

Copy link
Copy Markdown

Yeah.. it's painful. But in this case we're adding functional tests, so we've to run it.

@ericsnowcurrently
Eric Snow (ericsnowcurrently) merged commit 9a8e7ee into microsoft:master Jan 22, 2020
@ericsnowcurrently
Eric Snow (ericsnowcurrently) deleted the env-vars-functional-tests branch January 22, 2020 20:04
@lock lock Bot locked as resolved and limited conversation to collaborators Jan 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@karrtikr Kartik Raj (karrtikr) karrtikr approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

no-changelog No news entry required

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ericsnowcurrently @codecov-io @karrtikr