Skip to content
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

fix: Add unit test related to sshnpd #1263

Merged
merged 2 commits into from
Aug 19, 2024
Merged

fix: Add unit test related to sshnpd #1263

merged 2 commits into from
Aug 19, 2024

Conversation

sitaram-kalluri
Copy link
Member

@sitaram-kalluri sitaram-kalluri commented Aug 14, 2024

- What I did

  • Added a unit test to notify an ssh_request to sshnpd and assert the payload in the notification value.

- How I did it

  • When invoking the SshnpDartPureImpl.run() method, the method call await callInitialization(); initializes both the sshnpd and sshrvd channels, which leads to the creation a lot of unnecessary mock objects in the test scenario. To address this, the 'notify' call was moved to a separate method, sendSshRequestToSshnpd, to facilitate easier unit testing.
  • In the unit test,
    • Invoke sshnp.srvdChannel.initialize() which will fetch the host and ports from srvd.
    • Then, invoke sshnp.sendSshRequestToSshnpd() which will build the notification key - 'ssh_request` and payload as notification value which is sent to sshnpd.
    • Assert the payload in the mockNotificationService callback.

- How to verify it

  • All tests should pass.

Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

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

LGTM thank you @sitaram-kalluri

@sitaram-kalluri sitaram-kalluri merged commit 5eaaaff into trunk Aug 19, 2024
7 checks passed
@sitaram-kalluri sitaram-kalluri deleted the add_unit_tests branch August 19, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants