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

test: SshnpdChannel #587

Merged
merged 6 commits into from
Nov 23, 2023
Merged

test: SshnpdChannel #587

merged 6 commits into from
Nov 23, 2023

Conversation

XavierChanth
Copy link
Member

- What I did

  • Tested SshnpdChannel, skipped listDevices, will return to it later, since it isn't as urgent as the other tests that we still need

- How I did it

- How to verify it

- Description for the changelog
test: SshnpdChannel

@XavierChanth XavierChanth requested a review from gkc November 23, 2023 00:50
@@ -10,7 +10,7 @@ mixin class AsyncInitialization {
// * Public members

/// Used to check if initialization has started
bool get initalizeStarted => _initializeStarted;
bool get initializeStarted => _initializeStarted;

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed a typo

}

/// Main reponse handler for the daemon's notifications.
Future<void> _handleSshnpdResponses(AtNotification notification) async {
@visibleForTesting
Future<void> handleSshnpdResponses(AtNotification notification) async {
Copy link
Member Author

Choose a reason for hiding this comment

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

Scope: Private -> Visible for Testing

@@ -234,7 +235,8 @@ abstract class SshnpdChannel with AsyncInitialization, AtClientBindings {
}

/// A custom implementation of AtClient.getAtKeys which bypasses the cache
Future<List<AtKey>> _getAtKeysRemote(
@visibleForTesting
Future<List<AtKey>> getAtKeysRemote(
Copy link
Member Author

Choose a reason for hiding this comment

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

Scope: Private -> Visible for Testing

@@ -17,7 +17,7 @@ void main() {
late SshrvGeneratorStub<String> sshrvGeneratorStub;
late MockAtClient mockAtClient;
late StreamController<AtNotification> notificationStreamController;
late FunctionStub notifyStub;
late NotifyStub notifyStub;
late SubscribeStub subscribeStub;
Copy link
Member Author

Choose a reason for hiding this comment

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

Wasn't verifying the arguments passed into notify, so I created a NotifyStub class which allows me to validate those arguments

},
subscribe: ({regex, shouldDecrypt = false}) {
return subscribeStub();
},
Copy link
Member Author

@XavierChanth XavierChanth Nov 23, 2023

Choose a reason for hiding this comment

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

This removed function is now in a when(notificationInvocation).thenAnswer(...) block later on

),
),
any(),
),
Copy link
Member Author

Choose a reason for hiding this comment

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

Here's the better argument matching as mentioned, checking that the values contained in the key passed in are as expected


expect(stubbedSshrvdChannel.host, '234.234.234.234');
expect(stubbedSshrvdChannel.port, 135);
}); // test Initialization - non-sshrvd host

test('Initialization completes - sshrvd host', () async {
/// Set the required parameters
whenInitializationWithSshrvdHost();
Copy link
Member Author

Choose a reason for hiding this comment

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

Added an init completion test, which checks if completeInitialization() is called when callInitialization() is called ... this only needs to be tested in certain places, usually impl classes, but initialization only occurs in the base class for sshrvd channel, so we can call and test for it in the base class.

@gkc gkc merged commit d64e03c into trunk Nov 23, 2023
11 checks passed
@gkc gkc deleted the unit_tests_5 branch November 23, 2023 13:06
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.

2 participants