Skip to content

fix: Changes to consider only updatePassedSnapshot if updateSnapshot is also true#327

Merged
10xLaCroixDrinker merged 9 commits intoamericanexpress:mainfrom
goverdhan07:main
Jul 25, 2023
Merged

fix: Changes to consider only updatePassedSnapshot if updateSnapshot is also true#327
10xLaCroixDrinker merged 9 commits intoamericanexpress:mainfrom
goverdhan07:main

Conversation

@goverdhan07
Copy link
Copy Markdown
Contributor

@goverdhan07 goverdhan07 commented Mar 27, 2023 •
edited
Loading

Description

updatePassedSnapshot to only update "passed" snapshots when jest --updatesnapshot flag is present.

Motivation and Context

fixes: #325

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (adding or updating documentation)
  • Dependency update

Checklist:

  • My change requires a change to the documentation and I have updated the documentation accordingly.
  • My changes are in sync with the code style of this project.
  • There aren't any other open Pull Requests for the same issue/update.
  • These changes should be applied to a maintenance branch.
  • I have added the Apache 2.0 license header to any new files created.

@goverdhan07 goverdhan07 requested a review from a team as a code owner March 27, 2023 14:17
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 27, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@chetan1507
Copy link
Copy Markdown

chetan1507 commented Apr 2, 2023 •
edited
Loading

can we please merge this?
This is a very useful fix for CI detection in case of missing snapshots.

code-forger
code-forger reviewed Apr 4, 2023
View reviewed changes
Comment thread src/diff-snapshot.js
@10xLaCroixDrinker
Copy link
Copy Markdown
Contributor

10xLaCroixDrinker commented Apr 6, 2023

Please sign the CLA

@goverdhan07
Copy link
Copy Markdown
Contributor Author

goverdhan07 commented Apr 7, 2023

Please sign the CLA

hi @10xLaCroixDrinker I have signed the cla but it still says - not signed. Maybe the Github action is broken? please check and let me know.

@goverdhan07 goverdhan07 requested a review from code-forger May 21, 2023 08:26
@10xLaCroixDrinker
Copy link
Copy Markdown
Contributor

10xLaCroixDrinker commented Jun 27, 2023

For the CLA to work, you need to amend your commits to use the email associated with your GitHub account

@goverdhan07 goverdhan07 force-pushed the main branch 3 times, most recently from 5433bab to 5c55d3b Compare July 23, 2023 10:07
goverdhan07 and others added 6 commits July 23, 2023 15:47
@goverdhan07
chore(deps): bump qs from 6.5.2 to 6.5.3 (americanexpress#320)
2c724e0
@goverdhan07
chore(deps): make jest optional peerDependency (americanexpress#322)
83d2142
@goverdhan07 @dependabot
chore(deps): bump http-cache-semantics from 4.1.0 to 4.1.1 (americane…
da99760 
…xpress#324)

Bumps [http-cache-semantics](https://github.com/kornelski/http-cache-semantics) from 4.1.0 to 4.1.1.
- [Release notes](https://github.com/kornelski/http-cache-semantics/releases)
- [Commits](kornelski/http-cache-semantics@v4.1.0...v4.1.1)

---
updated-dependencies:
- dependency-name: http-cache-semantics
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@goverdhan07
fix: updatePassedSnapshot to only update passed snapshots when jest -…
e98d14e 
…-updatesnapshot flag is present
@goverdhan07
fix: fixed indentations
b933927
@goverdhan07
fix: added unit test case
71021bb
@goverdhan07
Copy link
Copy Markdown
Contributor Author

goverdhan07 commented Jul 23, 2023

@code-forger / @10xLaCroixDrinker Please re-review this PR. I have fixed CLA issue now.

10xLaCroixDrinker
10xLaCroixDrinker suggested changes Jul 24, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@10xLaCroixDrinker 10xLaCroixDrinker left a comment

Choose a reason for hiding this comment

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

Please restore the lockfile.

@10xLaCroixDrinker 10xLaCroixDrinker merged commit b9d9c3f into americanexpress:main Jul 25, 2023
oneamexbot added a commit that referenced this pull request Jul 25, 2023
@oneamexbot
chore(release): 6.1.1 [skip ci]
96878dc 
## [6.1.1](v6.1.0...v6.1.1) (2023-07-25)

### Bug Fixes

* only updatePassedSnapshot if updateSnapshot is also true ([#327](#327)) ([b9d9c3f](b9d9c3f)), closes [#320](#320) [#322](#322) [#324](#324)
@oneamexbot
Copy link
Copy Markdown
Contributor

oneamexbot commented Jul 25, 2023

🎉 This PR is included in version 6.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@code-forger code-forger Awaiting requested review from code-forger

2 more reviewers

@10xLaCroixDrinker 10xLaCroixDrinker 10xLaCroixDrinker approved these changes

@dogpatch626 dogpatch626 dogpatch626 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

one-app-team-review-requested released

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

updatePassedSnapshot updates all snapshot regardless of jest --updatesnapshot value

7 participants

@goverdhan07 @CLAassistant @chetan1507 @10xLaCroixDrinker @oneamexbot @code-forger @dogpatch626
⚡