Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Support deletion in process.env too #5466

Merged
merged 1 commit into from
Feb 5, 2018
Merged

Support deletion in process.env too #5466

merged 1 commit into from
Feb 5, 2018

Conversation

mjesun
Copy link
Contributor

@mjesun mjesun commented Feb 5, 2018

Get/Set behavior was implemented, but we lacked deletion. This PR adds it, along with some tests.

SimenB
SimenB reviewed Feb 5, 2018
View reviewed changes
packages/jest-util/src/__tests__/create_process_object.test.js Outdated
// You can delete through case-insensitiveness too.
delete fake.prop_string;

expect('PROP_STRING' in fake).toBe(false);
Copy link
Member

@SimenB SimenB Feb 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

expect(fake).not.toHaveProperty('PROP_STRING');

?

Not sure if better or not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that could work too 🙂

@mjesun mjesun requested review from cpojer and thymikee February 5, 2018 16:12
SimenB
SimenB reviewed Feb 5, 2018
View reviewed changes
CHANGELOG.md
@@ -2,12 +2,19 @@

### Features

* `[jest-util]` Add deletion to `process.env` as well
([#5466](https://github.com/facebook/jest/pull/5466))
* `[jest-util]` Add case-insensitive getters/setters to `process.env`
Copy link
Member

Choose a reason for hiding this comment

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

❤️

@SimenB
Copy link
Member

SimenB commented Feb 5, 2018

Lint failures on CI

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@d6781c3). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #5466   +/-   ##
=========================================
  Coverage          ?   62.23%           
=========================================
  Files             ?      205           
  Lines             ?     6932           
  Branches          ?        4           
=========================================
  Hits              ?     4314           
  Misses            ?     2617           
  Partials          ?        1
Impacted Files Coverage Δ
packages/jest-util/src/create_process_object.js 100% <ø> (ø)

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 d6781c3...fe43161. Read the comment docs.

@mjesun mjesun merged commit 5229f35 into jestjs:master Feb 5, 2018
@mjesun mjesun deleted the add-delete-to-process-env branch February 6, 2018 13:19
@renovate renovate bot mentioned this pull request Feb 14, 2018
Open
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers

@SimenB SimenB SimenB approved these changes

@cpojer cpojer Awaiting requested review from cpojer

@thymikee thymikee Awaiting requested review from thymikee

Assignees
No one assigned
Labels
CLA Signed ✔️
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mjesun @SimenB @codecov-io @facebook-github-bot