From 201fb9a913b583ca2c955298211b4ecde86486b6 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Mon, 11 Dec 2023 16:35:21 +0100 Subject: [PATCH 1/7] Add `testframework-rustfmt.yml` Check formatting of rust source code in the `test` workspace. --- .github/workflows/testframework-rustfmt.yml | 29 +++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 .github/workflows/testframework-rustfmt.yml diff --git a/.github/workflows/testframework-rustfmt.yml b/.github/workflows/testframework-rustfmt.yml new file mode 100644 index 000000000000..8889653183b9 --- /dev/null +++ b/.github/workflows/testframework-rustfmt.yml @@ -0,0 +1,29 @@ +# Run `cargo fmt --check` on the `test` workspace +--- +name: DES Testframework - Check formatting +on: + pull_request: + paths: + - 'test/**/*.rs' + - .github/workflows/rustfmt-test.yml + - rustfmt.toml + workflow_dispatch: +jobs: + check-formatting-test: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v3 + + - name: Install Rust + uses: actions-rs/toolchain@v1.0.6 + with: + toolchain: stable + components: rustfmt + default: true + + - name: Check formatting + working-directory: test + run: |- + rustfmt --version + cargo fmt -- --check From 95c5e9fda4aa77f7c41c60cefda0fb21f21b671a Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Mon, 11 Dec 2023 16:36:07 +0100 Subject: [PATCH 2/7] Add `testframework-clippy.yml` Check for code quality improvements on all rust source code with `clippy` in the `test` workspace. --- .github/workflows/testframework-clippy.yml | 73 ++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 .github/workflows/testframework-clippy.yml diff --git a/.github/workflows/testframework-clippy.yml b/.github/workflows/testframework-clippy.yml new file mode 100644 index 000000000000..670001c882cf --- /dev/null +++ b/.github/workflows/testframework-clippy.yml @@ -0,0 +1,73 @@ +# Run `clippy` on the `test` workspace +--- +name: DES Testframework - Clippy +on: + pull_request: + paths: + - 'test/**/*.rs' + - .github/workflows/clippy-test.yml + - clippy.toml + workflow_dispatch: +jobs: + clippy-check-test: + name: Clippy linting of test workspace + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest] + runs-on: ${{ matrix.os }} + steps: + - name: Checkout repository + uses: actions/checkout@v3 + + - name: Install Protoc + uses: arduino/setup-protoc@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + + - uses: actions-rs/toolchain@v1.0.6 + with: + toolchain: stable + components: clippy + override: true + + - name: Install build dependencies + if: matrix.os == 'ubuntu-latest' + run: | + sudo apt-get update + sudo apt-get install libdbus-1-dev + + - name: Clippy check + working-directory: test + shell: bash + env: + RUSTFLAGS: --deny warnings + run: | + time cargo clippy --locked -- -W clippy::unused_async + + clippy-check-test-windows: + name: Clippy linting of test workspace (Windows) + runs-on: windows-latest + steps: + - name: Checkout repository + uses: actions/checkout@v3 + + - name: Install Protoc + uses: arduino/setup-protoc@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + + - uses: actions-rs/toolchain@v1.0.6 + with: + toolchain: stable + components: clippy + override: true + + - name: Clippy check + working-directory: test + shell: bash + env: + RUSTFLAGS: --deny warnings + run: | + # Exclude checking test-manager on Windows, since it is not a supported compilation target. + time cargo clippy --workspace --exclude test-manager --locked -- -W clippy::unused_async From 8660367f774ebe37a646702f54124dc439f3a399 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Mon, 11 Dec 2023 16:36:42 +0100 Subject: [PATCH 3/7] Add `testframework.yml` Build the `test-manager` for the host platforms (Linux & macOS) as well as the `test-runner` for all supported platforms (Linux, Windows & macOS). --- .github/workflows/testframework.yml | 111 ++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 .github/workflows/testframework.yml diff --git a/.github/workflows/testframework.yml b/.github/workflows/testframework.yml new file mode 100644 index 000000000000..f382567c52cd --- /dev/null +++ b/.github/workflows/testframework.yml @@ -0,0 +1,111 @@ +# Compile the test framework +--- +name: DES Testframework - Build +on: + pull_request: + paths: + - '**' + - '!**/**.md' + - '!.github/workflows/**' + - '.github/workflows/daemon.yml' + - '!android/**' + - '!audits/**' + - '!build-apk.sh' + - '!build.sh' + - '!clippy.toml' + - '!deny.toml' + - '!docs/**' + - '!graphics/**' + - '!gui/**' + - '!ios/**' + - '!scripts/**' + - '!.*ignore' + - '!prepare-release.sh' + - '!rustfmt.toml' + - '!.yamllint' + workflow_dispatch: +jobs: + prepare-build-test-framework-linux: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v3 + + - name: Use custom container image if specified + if: ${{ github.event.inputs.override_container_image != '' }} + run: echo "inner_container_image=${{ github.event.inputs.override_container_image }}" + >> $GITHUB_ENV + + - name: Use default container image and resolve digest + if: ${{ github.event.inputs.override_container_image == '' }} + run: | + echo "inner_container_image=$(cat ./building/linux-container-image.txt)" >> $GITHUB_ENV + + outputs: + container_image: ${{ env.inner_container_image }} + + # Build the test runner + test manager at once. + build-test-framework-linux: + needs: prepare-build-test-framework-linux + runs-on: ubuntu-latest + container: + image: ${{ needs.prepare-build-test-framework-linux.outputs.container_image }} + + steps: + # Fix for HOME path overridden by GH runners when building in containers, see: + # https://github.com/actions/runner/issues/863 + - name: Fix HOME path + run: echo "HOME=/root" >> $GITHUB_ENV + + - name: Install system dependencies # Needed to build test-manager, and is not included in the app container. + run: apt update && apt install -y pkg-config libssl-dev libpcap-dev + + - name: Checkout repository + uses: actions/checkout@v3 + + - name: Build test framework + working-directory: test + run: cargo build --release + + # Build the test runner + test manager at once. + build-test-framework-macos: + runs-on: macos-latest + steps: + - name: Checkout repository + uses: actions/checkout@v2 + + - name: Install Protoc + uses: arduino/setup-protoc@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Install Rust + uses: actions-rs/toolchain@v1.0.6 + with: + toolchain: stable + default: true + + - name: Build test runner + working-directory: test + run: cargo build + + # Build only the test-runner binary on Windows. Windows is not a supported host for test-manager. + build-test-runner-windows: + # Cross-compile the test runner for Windows from Linux. + needs: prepare-build-test-framework-linux + runs-on: ubuntu-latest + container: + image: ${{ needs.prepare-build-test-framework-linux.outputs.container_image }} + continue-on-error: true + steps: + # Fix for HOME path overridden by GH runners when building in containers, see: + # https://github.com/actions/runner/issues/863 + - name: Fix HOME path + run: echo "HOME=/root" >> $GITHUB_ENV + + - name: Checkout repository + uses: actions/checkout@v3 + + - name: Build test runner + working-directory: test + run: cargo build --release -p test-runner --target x86_64-pc-windows-gnu From 90344642c8ec008a086a85de55144d64346701ed Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Tue, 12 Dec 2023 11:41:11 +0100 Subject: [PATCH 4/7] Exclude `test` workspace from triggering `android-app` workflow --- .github/workflows/android-app.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/android-app.yml b/.github/workflows/android-app.yml index 13d5c011a613..77e7266bfd85 100644 --- a/.github/workflows/android-app.yml +++ b/.github/workflows/android-app.yml @@ -13,6 +13,7 @@ on: - '!graphics/**' - '!gui/**' - '!ios/**' + - '!test/**' - '!scripts/**' - '!windows/**' - '!**/**.md' From 8686fc2792906c8e5cb0dcc49845efd7690f98fa Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Tue, 12 Dec 2023 10:16:18 +0100 Subject: [PATCH 5/7] Add missing system deps to `test/Dockerfile` Add some missing system libraries for compiling the test framework. These are dependencies which the base `mullvad/mullvadvpn-app-build` container does not include. --- test/Dockerfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/Dockerfile b/test/Dockerfile index 23dcdc4b5b7e..f2e28244baf2 100644 --- a/test/Dockerfile +++ b/test/Dockerfile @@ -1,2 +1,5 @@ ARG IMAGE=ghcr.io/mullvad/mullvadvpn-app-build:latest FROM $IMAGE + +RUN apt-get update && apt-get install -y \ + pkg-config libssl-dev libpcap-dev From 8a053ab50624892dbefcb6cd386aee650651a48b Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Mon, 11 Dec 2023 16:37:35 +0100 Subject: [PATCH 6/7] [Clippy] Fix unused `async` --- test/test-manager/src/mullvad_daemon.rs | 2 +- test/test-manager/src/run_tests.rs | 4 ++-- test/test-rpc/src/transport.rs | 2 +- test/test-runner/src/main.rs | 2 +- test/test-runner/src/net.rs | 4 ++-- test/test-runner/src/sys.rs | 1 + 6 files changed, 8 insertions(+), 7 deletions(-) diff --git a/test/test-manager/src/mullvad_daemon.rs b/test/test-manager/src/mullvad_daemon.rs index 2bfff38ddeba..e467940d5824 100644 --- a/test/test-manager/src/mullvad_daemon.rs +++ b/test/test-manager/src/mullvad_daemon.rs @@ -76,7 +76,7 @@ impl RpcClientProvider { } } -pub async fn new_rpc_client( +pub fn new_rpc_client( connection_handle: ConnectionHandle, mullvad_daemon_transport: GrpcForwarder, ) -> RpcClientProvider { diff --git a/test/test-manager/src/run_tests.rs b/test/test-manager/src/run_tests.rs index f0ff40203404..4a296ce2887c 100644 --- a/test/test-manager/src/run_tests.rs +++ b/test/test-manager/src/run_tests.rs @@ -35,7 +35,7 @@ pub async fn run( let serial_stream = tokio_serial::SerialStream::open(&tokio_serial::new(pty_path, BAUD)).unwrap(); let (runner_transport, mullvad_daemon_transport, mut connection_handle, completion_handle) = - test_rpc::transport::create_client_transports(serial_stream).await?; + test_rpc::transport::create_client_transports(serial_stream)?; if !skip_wait { connection_handle.wait_for_server().await?; @@ -45,7 +45,7 @@ pub async fn run( let client = ServiceClient::new(connection_handle.clone(), runner_transport); let mullvad_client = - mullvad_daemon::new_rpc_client(connection_handle, mullvad_daemon_transport).await; + mullvad_daemon::new_rpc_client(connection_handle, mullvad_daemon_transport); let mut tests: Vec<_> = inventory::iter::().collect(); tests.sort_by_key(|test| test.priority.unwrap_or(0)); diff --git a/test/test-rpc/src/transport.rs b/test/test-rpc/src/transport.rs index 6c8b7a7060ce..753420900dcc 100644 --- a/test/test-rpc/src/transport.rs +++ b/test/test-rpc/src/transport.rs @@ -162,7 +162,7 @@ pub fn create_server_transports( (runner_forwarder_1, daemon_rx, completion_handle) } -pub async fn create_client_transports( +pub fn create_client_transports( serial_stream: impl AsyncRead + AsyncWrite + Unpin + Send + 'static, ) -> Result< ( diff --git a/test/test-runner/src/main.rs b/test/test-runner/src/main.rs index fe85e6f89fe0..c67876526bb7 100644 --- a/test/test-runner/src/main.rs +++ b/test/test-runner/src/main.rs @@ -177,7 +177,7 @@ impl Service for TestServer { _: context::Context, interface: String, ) -> Result { - net::get_interface_ip(&interface).await + net::get_interface_ip(&interface) } async fn get_default_interface(self, _: context::Context) -> Result { diff --git a/test/test-runner/src/net.rs b/test/test-runner/src/net.rs index a4a7a2db47bd..1de728a7440c 100644 --- a/test/test-runner/src/net.rs +++ b/test/test-runner/src/net.rs @@ -202,7 +202,7 @@ pub async fn send_ping( } #[cfg(unix)] -pub async fn get_interface_ip(interface: &str) -> Result { +pub fn get_interface_ip(interface: &str) -> Result { // TODO: IPv6 use std::net::Ipv4Addr; @@ -225,7 +225,7 @@ pub async fn get_interface_ip(interface: &str) -> Result Result { +pub fn get_interface_ip(interface: &str) -> Result { // TODO: IPv6 get_interface_ip_for_family(interface, talpid_windows::net::AddressFamily::Ipv4) diff --git a/test/test-runner/src/sys.rs b/test/test-runner/src/sys.rs index 7363e38e7267..ecc767d3981d 100644 --- a/test/test-runner/src/sys.rs +++ b/test/test-runner/src/sys.rs @@ -379,6 +379,7 @@ pub async fn set_daemon_log_level(verbosity_level: Verbosity) -> Result<(), test } #[cfg(target_os = "macos")] +#[allow(clippy::unused_async)] pub async fn set_daemon_log_level(_verbosity_level: Verbosity) -> Result<(), test_rpc::Error> { // TODO: Not implemented log::warn!("Setting log level is not implemented on macOS"); From e14ccfe2075971b0be1dc63273911b128c46d5cf Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Mon, 11 Dec 2023 16:38:07 +0100 Subject: [PATCH 7/7] [Clippy] Fix complex types --- test/test-rpc/src/transport.rs | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/test/test-rpc/src/transport.rs b/test/test-rpc/src/transport.rs index 753420900dcc..291e08eb532e 100644 --- a/test/test-rpc/src/transport.rs +++ b/test/test-rpc/src/transport.rs @@ -120,16 +120,18 @@ impl ConnectionHandle { } } -pub fn create_server_transports( - serial_stream: impl AsyncRead + AsyncWrite + Unpin + Send + 'static, -) -> ( +type ServerTransports = ( tarpc::transport::channel::UnboundedChannel< ClientMessage, Response, >, GrpcForwarder, CompletionHandle, -) { +); + +pub fn create_server_transports( + serial_stream: impl AsyncRead + AsyncWrite + Unpin + Send + 'static, +) -> ServerTransports { let (runner_forwarder_1, runner_forwarder_2) = tarpc::transport::channel::unbounded(); let (daemon_rx, mullvad_daemon_forwarder) = tokio::io::duplex(DAEMON_CHANNEL_BUF_SIZE); @@ -164,18 +166,7 @@ pub fn create_server_transports( pub fn create_client_transports( serial_stream: impl AsyncRead + AsyncWrite + Unpin + Send + 'static, -) -> Result< - ( - tarpc::transport::channel::UnboundedChannel< - Response, - ClientMessage, - >, - GrpcForwarder, - ConnectionHandle, - CompletionHandle, - ), - Error, -> { +) -> Result { let (runner_forwarder_1, runner_forwarder_2) = tarpc::transport::channel::unbounded(); let (daemon_rx, mullvad_daemon_forwarder) = tokio::io::duplex(DAEMON_CHANNEL_BUF_SIZE); @@ -216,6 +207,16 @@ pub fn create_client_transports( )) } +type ClientTransports = ( + tarpc::transport::channel::UnboundedChannel< + Response, + ClientMessage, + >, + GrpcForwarder, + ConnectionHandle, + CompletionHandle, +); + #[derive(err_derive::Error, Debug)] #[error(no_from)] enum ForwardError {