-
Notifications
You must be signed in to change notification settings - Fork 36
Fix flaky TSO test and remove notify keyword from test JCL #386
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #386 +/- ##
==========================================
+ Coverage 81.86% 81.88% +0.01%
==========================================
Files 48 48
Lines 2768 2771 +3
==========================================
+ Hits 2266 2269 +3
Misses 502 502
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
a8e4324 to
6b1b9cc
Compare
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
329f01e to
151da20
Compare
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
151da20 to
52801dd
Compare
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.
Pull request overview
This PR fixes a flaky TSO test by improving the reliability of TSO command execution to align with Zowe Node SDKs behavior, and removes the "notify" keyword from test JCL to prevent spamming the test system with TSO notifications.
Key Changes
- Modified TSO command issuing to fetch and suppress startup messages before executing commands
- Added
read_replyparameter to thesend()method to control whether to wait for TSO session replies - Removed
NOTIFY=&SYSUIDfrom test JCL fixtures to prevent unnecessary system notifications
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/zos_tso/zowe/zos_tso_for_zowe_sdk/tso.py |
Enhanced issue_command method to fetch startup messages and added read_reply parameter to send() method for better control over TSO session behavior |
src/zos_tso/zowe/zos_tso_for_zowe_sdk/response/tso.py |
Made tsoData field optional in SendResponse to support responses without data when read_reply=False |
tests/unit/test_zos_tso.py |
Updated unit test mocks to reflect new TSO command flow with additional API calls for fetching startup messages |
tests/integration/fixtures/sample.jcl |
Removed NOTIFY=&SYSUID parameter from JCL job definition |
tests/integration/fixtures/jobs.json |
Removed NOTIFY=&SYSUID parameter from JCL code array |
CHANGELOG.md |
Documented the bug fix for TSO command output reliability issue |
.github/workflows/sdk-release.yaml |
Added apt-get update before package installation for better reliability |
.github/workflows/sdk-build.yml |
Added apt-get update before package installation for better reliability |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| list[dict[str, Any]] | ||
| A non-normalized list from TSO containing the result from the command | ||
| """ | ||
| return list(self.send(session_key, message).tsoData) |
Copilot
AI
Dec 28, 2025
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.
This method calls self.send(session_key, message).tsoData and wraps it with list(). Since tsoData field in SendResponse is now Optional[list[dict[str, Any]]] (can be None), calling list(None) will raise a TypeError if the API returns a response without tsoData. Consider handling the None case, for example: return list(self.send(session_key, message).tsoData or []).
| return list(self.send(session_key, message).tsoData) | |
| return list(self.send(session_key, message).tsoData or []) |
| read_reply: bool | ||
| Whether to read the reply from the TSO session |
Copilot
AI
Dec 28, 2025
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.
The documentation for the read_reply parameter only states "Whether to read the reply from the TSO session" which is vague. Consider expanding this to explain what happens when set to False (e.g., "When False, the command is sent without waiting for a reply, and tsoData in the response will be None").
What It Does
readReply=falsequery param on API call for sending command to TSO sessionapt-get installfailing to installlibsecretpackage in Linux CI buildsHow to Test
Review Checklist
I certify that I have:
Additional Comments