refactor: SSHNPParams unit tests preparation #539

XavierChanth · 2023-10-18T21:45:19Z

- What I did

Started writing unit tests (which some are also included in this PR)
Realized that I needed to replace most of the inheritance with composition of classes so I removed the hierarchy and instead:
- Split most of the logic in the SSHNP implementations to mixins and classes:
  - SshrvdChannel - responsible for all communication to sshrvd
  - SshnpdChannel - responsible for all communication to sshnpd (there are subclasses because the legacy client uses unsigned payloads)
  - SshnpInitialTunnelHandler - a mixin which handles starting the initial ssh tunnel for forward clients
  - SshnpSshKeyHandler - a wrapper around SshKeyUtil which helps to safely use the correct ssh key management approach depending on implementation type (important for CLI vs Flutter implementations)

Other changes:

Separated lifecycle methods to mixins
- AsyncInitialization and AsyncDisposal
  - Note: I think AsyncDisposal might not be written to correctly handle lifecycle, but I will change that when/if we need to.
Renamed any uppercase class name segments such as SSHKeyUtil or SSHNPBlahBlahImpl to SshKeyUtil or SshnpBlahBlahImpl
Removed the e2e installer tests since they are failing and we are going to be ditching that particular installer in the 4.0 release

- How I did it

- How to verify it

- Description for the changelog
refactor: SSHNPParams unit tests preparation

XavierChanth · 2023-10-18T21:48:02Z

packages/noports_core/lib/src/sshnp/sshnp_params/config_key_repository.dart

 import 'package:noports_core/src/common/default_args.dart';
 import 'package:noports_core/src/sshnp/sshnp_params/sshnp_params.dart';

 class ConfigKeyRepository {
-  static const String _keyPrefix = 'profile_';
-  static const String _configNamespace = 'profiles.${DefaultArgs.namespace}';
+  @visibleForTesting


Made visible for testing in the unit tests

XavierChanth · 2023-10-18T21:48:26Z

packages/noports_core/lib/src/sshnp/sshnp_params/sshnp_arg.dart

@@ -325,7 +325,7 @@ class SSHNPArg {
    allowed: SupportedSshClient.values.map((c) => c.toString()).toList(),
    parseWhen: ParseWhen.commandLine,
  );
-  static final ssHAlgorithmArg = SSHNPArg(
+  static final sshAlgorithmArg = SSHNPArg(


Renamed ssH -> ssh

XavierChanth · 2023-10-18T21:49:18Z

packages/noports_core/test/sshnp_params/sshnp_core_test.dart

@@ -0,0 +1 @@
+void main() {}


These are to be written in a later PR

XavierChanth · 2023-11-02T14:40:14Z

.github/workflows/unit_tests.yaml

+        run: dart analyze
+      - name: dart test
+        working-directory: packages/noports_core
+        run: dart test


setup dart, dart analyze, dart test noports_core

XavierChanth · 2023-11-06T18:03:44Z

I just have to call it out... I was prepared to rip my hair out over a billion merge conflicts with 4ca6441. The git renames have saved this PR!

XavierChanth · 2023-11-06T18:13:26Z

.github/workflows/end2end_tests.yaml

@@ -192,172 +192,6 @@ jobs:
          docker compose down

  # Test suite 2
-  # Installer tests (local vs installer)
-  e2e_installer_test:
-    # Don't run on forks (cause no secrets), don't run if dependebot (cause no secrets)


Removing these tests, since we don't need them for the next release

This reverts commit 6f068b0.

XavierChanth · 2023-11-09T16:34:54Z

@gkc the trunk tests are not building (as expected) but local-local is now building, so I think it is safe for us to review.

gkc

Looks good @XavierChanth ... I think there's an opportunity to refactor some duplicated code in the various SshnpCore subclasses but that can come in a follow-up PR

packages/noports_core/lib/src/sshnp/impl/sshnp_dart_pure_impl.dart

packages/noports_core/lib/src/sshnp/impl/sshnp_exec_local_impl.dart

XavierChanth added 6 commits October 10, 2023 18:26

test: add ParserType Unit tests

4a799e4

test: unit tests for sshnp_params

dcf6d48

Merge remote-tracking branch 'origin/trunk' into noports_core_unit_tests

8edd5d9

test: SSHNP_Params

362ed28

test: SSHNP_Params

42db854

test: setup unit tests action

048b642

XavierChanth changed the title ~~feat(DRAFT): Noports core unit tests~~ feat: SSHNPParams unit tests Oct 18, 2023

XavierChanth changed the title ~~feat: SSHNPParams unit tests~~ test: SSHNPParams unit tests Oct 18, 2023

XavierChanth commented Oct 18, 2023

View reviewed changes

XavierChanth added 8 commits October 18, 2023 17:50

chore: remove unecessary dep

63d1cb2

test: checkin sshnp_result_test

3873d70

test: SSHNPResult

3a6bd68

chore: restructure tests to match lib

54357c0

test: add version test

7a052c8

Merge remote-tracking branch 'origin/trunk' into noports_core_unit_tests

4e8d2c8

checkin: wip separation of concerns in sshnp class

951de34

refactor: new composition architecture

8f16edf

XavierChanth commented Nov 2, 2023

View reviewed changes

XavierChanth added 8 commits November 2, 2023 11:39

chore: cleanup sshnp imports

4d6738a

chore: clean up sshnp constructors

74ff3a3

fix: race condition caused by multiple instances of channels per class

603aa82

fix: comment out test lines that are known to fail

d210f68

fix: register fallback value for any call in tests

34cf40e

fix: use correct var in logger

45e18a3

Merge branch 'trunk' into noports_core_unit_tests

4ca6441

ci: remove e2e installer tests (installer will be EOL with V4)

7b1cdf9

XavierChanth requested a review from gkc November 6, 2023 18:04

XavierChanth requested review from VJag and cconstab November 6, 2023 18:04

XavierChanth changed the title ~~test: SSHNPParams unit tests~~ refactor: SSHNPParams unit tests preparation Nov 6, 2023

XavierChanth commented Nov 6, 2023

View reviewed changes

chore: change all caps to pascal case

adf5aee

XavierChanth marked this pull request as ready for review November 6, 2023 18:24

chore: remove unused line

6f068b0

XavierChanth force-pushed the noports_core_unit_tests branch from d580e1a to 6f068b0 Compare November 8, 2023 16:04

XavierChanth added 3 commits November 9, 2023 09:59

style: remove whitespace

7bc8d29

Revert "chore: remove unused line"

7142a45

This reverts commit 6f068b0.

chore: upgrade dependencies

5b49fa0

Merge remote-tracking branch 'origin/trunk' into noports_core_unit_tests

78693f4

gkc approved these changes Nov 10, 2023

View reviewed changes

packages/noports_core/lib/src/sshnp/impl/sshnp_dart_pure_impl.dart Show resolved Hide resolved

packages/noports_core/lib/src/sshnp/impl/sshnp_exec_local_impl.dart Show resolved Hide resolved

gkc merged commit ba56ff7 into trunk Nov 10, 2023
10 checks passed

gkc deleted the noports_core_unit_tests branch November 10, 2023 15:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: SSHNPParams unit tests preparation #539

refactor: SSHNPParams unit tests preparation #539

XavierChanth commented Oct 18, 2023 •

edited

Loading

XavierChanth Oct 18, 2023

XavierChanth Oct 18, 2023

XavierChanth Oct 18, 2023

XavierChanth Nov 2, 2023

XavierChanth commented Nov 6, 2023

XavierChanth Nov 6, 2023

XavierChanth commented Nov 9, 2023

gkc left a comment

refactor: SSHNPParams unit tests preparation #539

refactor: SSHNPParams unit tests preparation #539

Conversation

XavierChanth commented Oct 18, 2023 • edited Loading

XavierChanth Oct 18, 2023

Choose a reason for hiding this comment

XavierChanth Oct 18, 2023

Choose a reason for hiding this comment

XavierChanth Oct 18, 2023

Choose a reason for hiding this comment

XavierChanth Nov 2, 2023

Choose a reason for hiding this comment

XavierChanth commented Nov 6, 2023

XavierChanth Nov 6, 2023

Choose a reason for hiding this comment

XavierChanth commented Nov 9, 2023

gkc left a comment

Choose a reason for hiding this comment

XavierChanth commented Oct 18, 2023 •

edited

Loading