test: re-enable integration tests for node manager #1695
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| OsString::from("--port"), | ||
| OsString::from("8080"), | ||
| OsString::from("--address"), | ||
| OsString::from("127.0.0.1"), |
Check notice
Code scanning / devskim
Accessing localhost could indicate debug code, or could hinder scaling. Note
| eq(ServiceInstallCtx { | ||
| args: vec
jacderida
force-pushed
the
node_man-test-re_enable_integration_tests
branch
4 times, most recently
from
53f3267
to
8c024fa
Compare
jacderida
force-pushed
the
node_man-test-re_enable_integration_tests
branch
2 times, most recently
from
5798e9f
to
3e28ab5
Compare
The status of services and the status of local networks are separated into two different commands in anticipation of the node manager integration tests running against a local network.
jacderida
force-pushed
the
node_man-test-re_enable_integration_tests
branch
3 times, most recently
from
70a33a9
to
307fc05
Compare
These tests are enabled again, along with some changes to the setup. Here are the important points: * The tests now run against an isolated local network and binary built during CI is supplied. * The workflow runs the e2e tests for both system-wide and user-mode services. * The test definitions are moved to a new workflow file, but it has the same conditions as the merge workflow. The reason is just because the merge workflow file is already large and difficult to navigate. * The upgrade integration tests are removed because since the service management refactor, unit tests cover the upgrade scenarios well. What we are more concerned with in the upgrade process is the logic of how different upgrade scenarios are handled. * The daemon integration tests are removed. The test that was setup was really trying to cover the scenario where peer retention was specified for restart commands, but we have agreed that the semantics of this command is wrong and that it needs to be broken down. In general, the daemon commands will correspond to the operations of the node manager, and the node manager operations should already be quite well tested. * Some documentation.
jacderida
force-pushed
the
node_man-test-re_enable_integration_tests
branch
from
307fc05
to
ec46a92
Compare
jacderida
changed the title
eaa56fa feat: provide
local statuscommandThe status of services and the status of local networks are separated into two different commands in
anticipation of the node manager integration tests running against a local network.
ec46a92 test: enable node man integration tests
These tests are enabled again, along with some changes to the setup.
Here are the important points:
workflow. The reason is just because the merge workflow file is already large and difficult to
navigate.
tests cover the upgrade scenarios well. What we are more concerned with in the upgrade process is
the logic of how different upgrade scenarios are handled.
scenario where peer retention was specified for restart commands, but we have agreed that the
semantics of this command is wrong and that it needs to be broken down. In general, the daemon
commands will correspond to the operations of the node manager, and the node manager operations
should already be quite well tested.
Copilot Summary
This pull request includes changes to the GitHub Actions workflows, Justfile, README, Vagrantfile, and several Rust source files in the
sn_node_managerdirectory. The changes mainly involve the addition of new workflows and tests, the modification of existing workflows and tests, and the refactoring of code for improved readability and maintainability.Modifications to GitHub Actions workflows:
.github/workflows/merge.yml: Removed thenode-manager-unit-testsjob and several commented out jobs..github/workflows/node_man_tests.yml: Added a new workflow for Node Manager tests. This workflow includes thenode-manager-unit-tests,node-manager-user-mode-e2e-tests, andnode-manager-e2e-testsjobs.Changes to Rust source files:
sn_node_manager/src/cmd/local.rs: Added a newstatusfunction for getting the status of the local network.sn_node_manager/src/cmd/node.rs: Removed redundant code from thestatusfunction that was moved tolocal.rs.Addition of new tests:
Justfile: Added a newnode-man-integration-teststarget for running integration tests in a local setup.sn_node_manager/README.md: Added instructions for running integration tests using a virtual machine provided by aVagrantfile.Modifications to the development environment:
sn_node_manager/Vagrantfile: Made several changes to the configuration of the virtual machine, including changing the synced folder, adding a new provisioner for copying thesafe_networkcode to the root user's home directory, and copying the.vimrcfile to the root user's home directory. [1] [2] [3]