From 2f19d9c7ce0e7d367f6384e31fd3b9bc4aea6567 Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Thu, 18 Nov 2021 14:51:09 -0800 Subject: [PATCH] ci: Fix coverage workflow (#1382) * Avoid building the binary target, since it slows down the build * Use --skip-clean to use the cached build * Fetch a pre-build tarpaulin binary (saves ~5m) * Rename `flaky_tests` feature to `flakey-in-ci` * Add a `flakey-in-coverage` feature * Mark broken tests with `ignore` (not feature-gated) * Enable all tests by default --- .github/workflows/coverage.yml | 12 +++++++++--- .github/workflows/slow.yml | 12 ++++++++++-- Makefile | 4 ---- linkerd/app/integration/Cargo.toml | 4 +++- linkerd/app/integration/src/tests/identity.rs | 9 ++++++--- linkerd/app/integration/src/tests/tap.rs | 8 ++++---- linkerd/app/integration/src/tests/telemetry.rs | 15 +++++++-------- .../src/tests/telemetry/tcp_errors.rs | 2 ++ .../app/integration/src/tests/transparency.rs | 16 +++++++++------- linkerd/app/test/Cargo.toml | 9 +-------- 10 files changed, 51 insertions(+), 40 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index cca61413bf..ce964c919a 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -9,6 +9,7 @@ on: env: CARGO_INCREMENTAL: 0 CARGO_NET_RETRY: 10 + CARGO_TARPAULIN_VERSION: 0.18.5 RUST_BACKTRACE: short RUSTUP_MAX_RETRIES: 10 @@ -26,7 +27,12 @@ jobs: steps: - run: apt update && apt install -y cmake clang golang # for boring - uses: actions/checkout@ec3a7ce113134d7a93b817d10a8272cb61118579 - - run: cargo install cargo-tarpaulin - - run: cargo tarpaulin --verbose --workspace --no-run - - run: cargo tarpaulin --verbose --workspace --out Xml --ignore-tests --no-fail-fast + - name: install cargo-tarpaulin ${{ env.CARGO_TARPAULIN_VERSION }} + run: | + cd "${CARGO_HOME}/bin" + curl -sL https://github.com/xd009642/tarpaulin/releases/download/${CARGO_TARPAULIN_VERSION}/cargo-tarpaulin-${CARGO_TARPAULIN_VERSION}-travis.tar.gz | tar xzvf - + - run: cargo tarpaulin --locked --workspace --exclude=linkerd2-proxy --exclude=linkerd-app-integration --no-run + - run: cargo tarpaulin --locked --workspace --exclude=linkerd2-proxy --exclude=linkerd-app-integration --skip-clean --ignore-tests --no-fail-fast --out=Xml + - run: cargo tarpaulin --locked --workspace --package=linkerd-app-integration --no-default-features --skip-clean --no-run + - run: cargo tarpaulin --locked --workspace --package=linkerd-app-integration --no-default-features --skip-clean --ignore-tests --no-fail-fast --out=Xml - uses: codecov/codecov-action@f32b3a3741e1053eb607407145bc9619351dc93b diff --git a/.github/workflows/slow.yml b/.github/workflows/slow.yml index ed7e77b782..a91c8fd252 100644 --- a/.github/workflows/slow.yml +++ b/.github/workflows/slow.yml @@ -61,7 +61,6 @@ jobs: --package=linkerd-app-core \ --package=linkerd-app-gateway \ --package=linkerd-app-inbound \ - --package=linkerd-app-integration \ --package=linkerd-app-outbound \ --package=linkerd-app-test - run: | @@ -71,7 +70,16 @@ jobs: --package=linkerd-app-core \ --package=linkerd-app-gateway \ --package=linkerd-app-inbound \ - --package=linkerd-app-integration \ --package=linkerd-app-outbound \ --package=linkerd-app-test + # Integration: enable tests that are flakey in coverage, but disable tests + # that can be flakey in CI... + - run: | + cargo test --no-run \ + --package=linkerd-app-integration \ + --no-default-features --features=flakey-in-coverage + - run: | + cargo test \ + --package=linkerd-app-integration \ + --no-default-features --features=flakey-in-coverage diff --git a/Makefile b/Makefile index b95d67bd63..289964ed8a 100644 --- a/Makefile +++ b/Makefile @@ -103,10 +103,6 @@ shellcheck: test: fetch $(CARGO_TEST) -.PHONY: test-flakey -test-flakey: fetch - $(CARGO_TEST) --features linkerd-app-integration/flaky_tests - .PHONY: package package: $(PKG_ROOT)/$(PKG) diff --git a/linkerd/app/integration/Cargo.toml b/linkerd/app/integration/Cargo.toml index 629eb6d6ac..ecac010f85 100644 --- a/linkerd/app/integration/Cargo.toml +++ b/linkerd/app/integration/Cargo.toml @@ -13,7 +13,9 @@ a dedicated crate to help the compiler cache dependencies properly. """ [features] -flaky_tests = [] # Disable to skip certain tests that should not be run on CI. +default = ["flakey-in-ci"] +flakey-in-ci = ["flakey-in-coverage"] +flakey-in-coverage = [] rustfmt = ["linkerd2-proxy-api/rustfmt"] [dependencies] diff --git a/linkerd/app/integration/src/tests/identity.rs b/linkerd/app/integration/src/tests/identity.rs index 78ea05fd31..f8cb3e9c63 100644 --- a/linkerd/app/integration/src/tests/identity.rs +++ b/linkerd/app/integration/src/tests/identity.rs @@ -245,8 +245,9 @@ mod require_id_header { server: $make_server:path, tls_server: $make_tls_server:path, ) => { + // FIXME(ver) this test was marked flakey, but now it consistently fails. + #[ignore] #[tokio::test] - #[cfg_attr(not(feature = "flaky_tests"), ignore)] async fn orig_dst_client_connects_to_tls_server() { let _trace = trace_init(); @@ -313,8 +314,9 @@ mod require_id_header { ); } + // FIXME(ver) this test was marked flakey, but now it consistently fails. + #[ignore] #[tokio::test] - #[cfg_attr(not(feature = "flaky_tests"), ignore)] async fn disco_client_connects_to_tls_server() { let _trace = trace_init(); @@ -391,8 +393,9 @@ mod require_id_header { ); } + // FIXME(ver) this test was marked flakey, but now it consistently fails. + #[ignore] #[tokio::test] - #[cfg_attr(not(feature = "flaky_tests"), ignore)] async fn orig_dst_client_cannot_connect_to_plaintext_server() { let _trace = trace_init(); diff --git a/linkerd/app/integration/src/tests/tap.rs b/linkerd/app/integration/src/tests/tap.rs index ee879c1d61..5c772ead8d 100644 --- a/linkerd/app/integration/src/tests/tap.rs +++ b/linkerd/app/integration/src/tests/tap.rs @@ -74,10 +74,9 @@ async fn rejects_incorrect_identity_when_identity_is_expected() { assert!(events.next().await.expect("next1").is_err()); } -// Flaky: sometimes the admin thread hasn't had a chance to register -// the Taps before the `client.get` is called. +// FIXME(ver) this test was marked flakey, but now it consistently fails. +#[ignore] #[tokio::test] -#[cfg_attr(not(feature = "flaky_tests"), ignore)] async fn inbound_http1() { let _trace = trace_init(); @@ -158,8 +157,9 @@ async fn inbound_http1() { assert_eq!(ev3.response_end_bytes(), 5); } +// FIXME(ver) this test was marked flakey, but now it consistently fails. +#[ignore] #[tokio::test] -#[cfg_attr(not(feature = "flaky_tests"), ignore)] async fn grpc_headers_end() { let _trace = trace_init(); diff --git a/linkerd/app/integration/src/tests/telemetry.rs b/linkerd/app/integration/src/tests/telemetry.rs index 84b4140c5b..17aab71ea7 100644 --- a/linkerd/app/integration/src/tests/telemetry.rs +++ b/linkerd/app/integration/src/tests/telemetry.rs @@ -493,7 +493,7 @@ where // Eventually, we can add some kind of mock timer system for simulating latency // more reliably, and re-enable this test. #[tokio::test] -#[cfg_attr(not(feature = "flaky_tests"), ignore)] +#[cfg_attr(not(feature = "flakey-in-ci"), ignore)] async fn inbound_response_latency() { test_response_latency(Fixture::inbound_with_server).await } @@ -503,7 +503,7 @@ async fn inbound_response_latency() { // Eventually, we can add some kind of mock timer system for simulating latency // more reliably, and re-enable this test. #[tokio::test] -#[cfg_attr(not(feature = "flaky_tests"), ignore)] +#[cfg_attr(not(feature = "flakey-in-ci"), ignore)] async fn outbound_response_latency() { test_response_latency(Fixture::outbound_with_server).await } @@ -676,7 +676,7 @@ mod outbound_dst_labels { // the mock controller before the first request has finished. // See linkerd/linkerd2#751 #[tokio::test] - #[cfg_attr(not(feature = "flaky_tests"), ignore)] + #[cfg_attr(not(feature = "flakey-in-ci"), ignore)] async fn controller_updates_addr_labels() { let _trace = trace_init(); info!("running test server"); @@ -755,12 +755,9 @@ mod outbound_dst_labels { } } - // Ignore this test on CI, as it may fail due to the reduced concurrency - // on CI containers causing the proxy to see both label updates from - // the mock controller before the first request has finished. - // See linkerd/linkerd2#751 + // FIXME(ver) this test was marked flakey, but now it consistently fails. + #[ignore] #[tokio::test] - #[cfg_attr(not(feature = "flaky_tests"), ignore)] async fn controller_updates_set_labels() { let _trace = trace_init(); info!("running test server"); @@ -1191,6 +1188,7 @@ mod transport { test_tcp_connect(TcpFixture::outbound()).await; } + #[cfg_attr(not(feature = "flakey-in-coverage"), ignore)] #[tokio::test] async fn outbound_tcp_accept() { test_tcp_accept(TcpFixture::outbound()).await; @@ -1294,6 +1292,7 @@ mod transport { test_read_bytes_total(TcpFixture::outbound()).await } + #[cfg_attr(not(feature = "flakey-in-coverage"), ignore)] #[tokio::test] async fn outbound_tcp_open_connections() { test_tcp_open_conns(TcpFixture::outbound()).await diff --git a/linkerd/app/integration/src/tests/telemetry/tcp_errors.rs b/linkerd/app/integration/src/tests/telemetry/tcp_errors.rs index 0dc3cd6d97..c138bc9ffc 100644 --- a/linkerd/app/integration/src/tests/telemetry/tcp_errors.rs +++ b/linkerd/app/integration/src/tests/telemetry/tcp_errors.rs @@ -231,6 +231,7 @@ async fn inbound_multi() { } /// Tests that TLS detect failure metrics are collected for the direct stack. +#[cfg_attr(not(feature = "flakey-in-coverage"), ignore)] #[tokio::test] async fn inbound_direct_multi() { let _trace = trace_init(); @@ -314,6 +315,7 @@ async fn inbound_invalid_ip() { /// Tests that the detect metric is not incremented when TLS is successfully /// detected by the direct stack. +#[cfg_attr(not(feature = "flakey-in-coverage"), ignore)] #[tokio::test] async fn inbound_direct_success() { let _trace = trace_init(); diff --git a/linkerd/app/integration/src/tests/transparency.rs b/linkerd/app/integration/src/tests/transparency.rs index bae28262c9..3028d89057 100644 --- a/linkerd/app/integration/src/tests/transparency.rs +++ b/linkerd/app/integration/src/tests/transparency.rs @@ -153,8 +153,9 @@ async fn inbound_tcp() { proxy.join_servers().await; } +// FIXME(ver) this test was marked flakey, but now it consistently fails. +#[ignore] #[tokio::test] -#[cfg_attr(not(feature = "flaky_tests"), ignore)] async fn loop_outbound_http1() { let _trace = trace_init(); @@ -174,8 +175,9 @@ async fn loop_outbound_http1() { assert_eq!(rsp.status(), http::StatusCode::BAD_GATEWAY); } +// FIXME(ver) this test was marked flakey, but now it consistently fails. +#[ignore] #[tokio::test] -#[cfg_attr(not(feature = "flaky_tests"), ignore)] async fn loop_inbound_http1() { let _trace = trace_init(); @@ -245,6 +247,8 @@ async fn tcp_server_first() { test_server_speaks_first(TestEnv::default()).await; } +// FIXME(ver) this test doesn't actually test TLS functionality. +#[ignore] #[tokio::test] async fn tcp_server_first_tls() { use std::path::PathBuf; @@ -289,18 +293,18 @@ async fn tcp_server_first_tls() { } #[tokio::test] -#[allow(warnings)] async fn tcp_connections_close_if_client_closes() { let _trace = trace_init(); let msg1 = "custom tcp hello\n"; let msg2 = "custom tcp bye"; - let (mut tx, mut rx) = mpsc::channel(1); + let (tx, mut rx) = mpsc::channel::<()>(1); let srv = server::tcp() .accept_fut(move |mut sock| { async move { + let _tx = tx; let mut vec = vec![0; 1024]; let n = sock.read(&mut vec).await?; @@ -309,10 +313,8 @@ async fn tcp_connections_close_if_client_closes() { let n = sock.read(&mut [0; 16]).await?; assert_eq!(n, 0); panic!("lol"); - tx.send(()).await.unwrap(); - Ok::<(), io::Error>(()) } - .map(|res| res.expect("TCP server must not fail")) + .map(|res: io::Result<()>| res.expect("TCP server must not fail")) }) .run() .await; diff --git a/linkerd/app/test/Cargo.toml b/linkerd/app/test/Cargo.toml index ed0a5ccb51..2a7f0061e6 100644 --- a/linkerd/app/test/Cargo.toml +++ b/linkerd/app/test/Cargo.toml @@ -6,16 +6,9 @@ license = "Apache-2.0" edition = "2021" publish = false description = """ -Proxy integration tests - -The test utilities can be very costly to compile, so they are extracted into -a dedicated crate to help the compiler cache dependencies properly. +Proxy test helpers """ -[features] -# Disable to skip certain tests that should not be run on CI. -flaky_tests = [] - [dependencies] futures = { version = "0.3", default-features = false } h2 = "0.3"