-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Port #3440 to release/3.1 #3465
Conversation
* Fix zowe#3422 Signed-off-by: JWaters02 <[email protected]> chore: changelog Signed-off-by: JWaters02 <[email protected]> Fix failing tests This is due to adding a new message box to say that operation was cancelled, and tests were still expecting no message Signed-off-by: JWaters02 <[email protected]> * Fix that integrated terminals were not opening Signed-off-by: JWaters02 <[email protected]> --------- Signed-off-by: JWaters02 <[email protected]> Signed-off-by: Fernando Rijo Cedeno <[email protected]> Co-authored-by: Fernando Rijo Cedeno <[email protected]> Co-authored-by: Billie Simmons <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release/3.1 #3465 +/- ##
============================================
Coverage 93.21% 93.21%
============================================
Files 120 120
Lines 12521 12524 +3
Branches 2831 2772 -59
============================================
+ Hits 11671 11674 +3
Misses 849 849
Partials 1 1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! thanks for porting the fix @JWaters02
Signed-off-by: Fernando Rijo Cedeno <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 😋
Thanks for porting! 🙏
Hey @JWaters02, as a reminder... you should be able to work out of direct branches instead of a fork 🙏
const showInformationMessage = jest.fn(); | ||
Object.defineProperty(vscode.window, "showInformationMessage", { value: showInformationMessage }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't catch this in the original PR - but when possible, please use jest.replaceProperty
or jest.spyOn
to avoid re-defining the property for other test suites 😋 this can be kept as-is since we (unfortunately) already do this in several places
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Trae I'll bear it in mind.
Currently working on a random side project using Jest to teach me about it, as even just trying to understand it by looking at examples and reading some docs is proving difficult. So hopefully my ability to work on the tests in the future will be much better - trying to write new test suite for new feature is basically impossible task for me right now 😅
Something to keep in mind 🙏 |
Proposed changes
Ports #3440 to the release/3.1 branch in preparation for a 3.1.2 patch release.
Release Notes
Milestone: 3.1.2
Changelog: Fixed an issue where cancelling Unix, MVS or TSO command still submits the command.
Types of changes
Checklist
General
yarn workspace vscode-extension-for-zowe vscode:prepublish
pnpm --filter vscode-extension-for-zowe vscode:prepublish
Code coverage
Deployment
Further comments