From 9c55ae618e186915879e54b5785fc0dcc8c0e9b8 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 22 Dec 2021 11:28:07 -0800 Subject: [PATCH 01/11] try to save test outputs from GitHub Actions --- .github/workflows/rust.yml | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 148cf9391bf..7d7eb8e7aca 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -82,6 +82,8 @@ jobs: run: cargo doc build-and-test: + env: + OMICRON_TMP=/tmp/omicron_tmp needs: skip_duplicate_jobs if: ${{ needs.skip_duplicate_jobs.outputs.should_skip != 'true' }} runs-on: ${{ matrix.os }} @@ -114,6 +116,8 @@ jobs: - name: Download CockroachDB binary if: steps.cache-cockroachdb.outputs.cache-hit != 'true' run: bash ./tools/ci_download_cockroachdb + - name: Create temporary directory for test outputs + run: mkdir -p $OMICRON_TMP - name: Build # We build with: # - RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings": disallow warnings @@ -125,10 +129,23 @@ jobs: # also gives us a record of which dependencies were used for each CI # run. Building with `--locked` ensures that the checked-in Cargo.lock # is up to date. - run: PATH="$PATH:$PWD/cockroachdb/bin:$PWD/clickhouse" RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings" cargo build --locked --all-targets --verbose + # - TMPDIR=$OMICRON_TMP: we specify a specific temporary directory so that + # failed test outputs will be in a known place that we can grab at the + # end without also grabbing random other temporary files. + run: TMPDIR=$OMICRON_TMP PATH="$PATH:$PWD/cockroachdb/bin:$PWD/clickhouse" RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings" cargo build --locked --all-targets --verbose - name: Run tests # Use the same RUSTFLAGS and RUSTDOCFLAGS as above to avoid having to # rebuild here. # Put "./cockroachdb/bin" and "./clickhouse" on the PATH for the test # suite. - run: PATH="$PATH:$PWD/cockroachdb/bin:$PWD/clickhouse" RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings" cargo test --workspace --locked --verbose + run: TMPDIR=$OMICRON_TMP PATH="$PATH:$PWD/cockroachdb/bin:$PWD/clickhouse" RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings" cargo test --workspace --locked --verbose + - name: Archive any failed test results + # actions/upload-artifact@v2.3.1 + uses: actions/upload-artifact@82c141cc518b40d92cc801eee768e7aafc9c2fa2 + with: + name: test_outputs + path: | + ${{ env.OMICRON_TMP }} + !${{ env.OMICRON_TMP }}/crdb-base + !${{ env.OMICRON_TMP }}/rustc* + retention-days: 7 From 88610c94cbcad495cbc19e0f25a3c6640ae8104e Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 22 Dec 2021 11:30:46 -0800 Subject: [PATCH 02/11] induce test failure to see what we get --- nexus/src/authz/context.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nexus/src/authz/context.rs b/nexus/src/authz/context.rs index cfd0fd45740..49cf26fb1ca 100644 --- a/nexus/src/authz/context.rs +++ b/nexus/src/authz/context.rs @@ -189,7 +189,7 @@ mod test { #[tokio::test] async fn test_organization() { - let logctx = dev::test_setup_log("test_database"); + let logctx = dev::test_setup_log("test_organization"); let mut db = dev::test_setup_database(&logctx.log).await; let (opctx, datastore) = crate::db::datastore::datastore_test(&logctx, &db).await; @@ -224,6 +224,7 @@ mod test { "expected unauthenticated user not to be able \ to create organization", ); + panic!("injected error (dap)"); db.cleanup().await.unwrap(); logctx.cleanup_successful(); } From b10dcfbd237d5964d379ed6e72c638697a2027a6 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 22 Dec 2021 11:54:17 -0800 Subject: [PATCH 03/11] fix build warning --- nexus/src/authz/context.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/nexus/src/authz/context.rs b/nexus/src/authz/context.rs index 49cf26fb1ca..e267543c9fe 100644 --- a/nexus/src/authz/context.rs +++ b/nexus/src/authz/context.rs @@ -224,6 +224,7 @@ mod test { "expected unauthenticated user not to be able \ to create organization", ); + #[allow(unreachable_code)] panic!("injected error (dap)"); db.cleanup().await.unwrap(); logctx.cleanup_successful(); From 17f5eadb1387bb1ab5fc233f3370e86b6c3b8f9a Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 22 Dec 2021 11:58:29 -0800 Subject: [PATCH 04/11] make sure we upload the artifacts on failure --- .github/workflows/rust.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 7d7eb8e7aca..11617a72766 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -140,6 +140,7 @@ jobs: # suite. run: TMPDIR=$OMICRON_TMP PATH="$PATH:$PWD/cockroachdb/bin:$PWD/clickhouse" RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings" cargo test --workspace --locked --verbose - name: Archive any failed test results + if: ${{ failure() }} # actions/upload-artifact@v2.3.1 uses: actions/upload-artifact@82c141cc518b40d92cc801eee768e7aafc9c2fa2 with: From f5255cc8b6731141a248143db57146e5d1108c98 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 22 Dec 2021 12:45:26 -0800 Subject: [PATCH 05/11] more build problems --- nexus/src/authz/context.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/nexus/src/authz/context.rs b/nexus/src/authz/context.rs index e267543c9fe..02850ad92c0 100644 --- a/nexus/src/authz/context.rs +++ b/nexus/src/authz/context.rs @@ -224,9 +224,12 @@ mod test { "expected unauthenticated user not to be able \ to create organization", ); - #[allow(unreachable_code)] - panic!("injected error (dap)"); + dummy(); db.cleanup().await.unwrap(); logctx.cleanup_successful(); } + + fn dummy() { + panic!("injected error (dap)"); + } } From 09405d6f58a7bf268b55e5547460b560c8584770 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 22 Dec 2021 13:00:04 -0800 Subject: [PATCH 06/11] fix rust.yml --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 11617a72766..611c93bb0a7 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -83,7 +83,7 @@ jobs: build-and-test: env: - OMICRON_TMP=/tmp/omicron_tmp + OMICRON_TMP: /tmp/omicron_tmp needs: skip_duplicate_jobs if: ${{ needs.skip_duplicate_jobs.outputs.should_skip != 'true' }} runs-on: ${{ matrix.os }} From 6a95893f007b1b6b2b5552e0367ebd280b27e77a Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 22 Dec 2021 14:32:47 -0800 Subject: [PATCH 07/11] mistake in rust.yml --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 611c93bb0a7..582929717f0 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -145,8 +145,8 @@ jobs: uses: actions/upload-artifact@82c141cc518b40d92cc801eee768e7aafc9c2fa2 with: name: test_outputs + retention-days: 7 path: | ${{ env.OMICRON_TMP }} !${{ env.OMICRON_TMP }}/crdb-base !${{ env.OMICRON_TMP }}/rustc* - retention-days: 7 From 69b2bf03f220bfb160aab2f331b214079b1cb53b Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 22 Dec 2021 16:53:13 -0800 Subject: [PATCH 08/11] do not clean up database on failure --- test-utils/src/dev/db.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test-utils/src/dev/db.rs b/test-utils/src/dev/db.rs index 88361210bfa..83236ca8a9d 100644 --- a/test-utils/src/dev/db.rs +++ b/test-utils/src/dev/db.rs @@ -572,7 +572,14 @@ impl Drop for CockroachInstance { } #[allow(unused_must_use)] if let Some(temp_dir) = self.temp_dir.take() { - temp_dir.close(); + /* + * Do NOT clean up the temporary directory in this case. + */ + let path = temp_dir.into_path(); + eprintln!( + "WARN: temporary directory leaked: {}", + path.display() + ); } } } From 8a9c27ddd6c11aa5eba86306003c72a89d0a86ab Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Thu, 23 Dec 2021 08:54:27 -0800 Subject: [PATCH 09/11] use a separate name for different GitHub runners --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 582929717f0..e588a9d279d 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -144,7 +144,7 @@ jobs: # actions/upload-artifact@v2.3.1 uses: actions/upload-artifact@82c141cc518b40d92cc801eee768e7aafc9c2fa2 with: - name: test_outputs + name: failed_test_outputs_${{ runner.os }} retention-days: 7 path: | ${{ env.OMICRON_TMP }} From a76cfcb3d58cb53947780e56feb4cc306ce0b375 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Thu, 23 Dec 2021 09:28:33 -0800 Subject: [PATCH 10/11] test_database_concurrent leaks databases --- test-utils/src/dev/db.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test-utils/src/dev/db.rs b/test-utils/src/dev/db.rs index 83236ca8a9d..4268d3e22da 100644 --- a/test-utils/src/dev/db.rs +++ b/test-utils/src/dev/db.rs @@ -1246,13 +1246,13 @@ mod test { */ #[tokio::test] async fn test_database_concurrent() { - let db1 = new_builder() + let mut db1 = new_builder() .build() .expect("failed to create starter for the first database") .start() .await .expect("failed to start first database"); - let db2 = new_builder() + let mut db2 = new_builder() .build() .expect("failed to create starter for the second database") .start() @@ -1281,6 +1281,9 @@ mod test { client2.query("SELECT v FROM foo", &[]).await.expect("list rows"); assert_eq!(rows.len(), 0); client2.cleanup().await.expect("second connection closed ungracefully"); + + db1.cleanup().await.expect("failed to clean up first database"); + db2.cleanup().await.expect("failed to clean up second database"); } /* Success case for make_pg_config() */ From 69b367121782650819c8d47956050003560f2152 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Thu, 23 Dec 2021 09:44:15 -0800 Subject: [PATCH 11/11] remove injected error --- nexus/src/authz/context.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/nexus/src/authz/context.rs b/nexus/src/authz/context.rs index 02850ad92c0..8d5db942df6 100644 --- a/nexus/src/authz/context.rs +++ b/nexus/src/authz/context.rs @@ -224,12 +224,7 @@ mod test { "expected unauthenticated user not to be able \ to create organization", ); - dummy(); db.cleanup().await.unwrap(); logctx.cleanup_successful(); } - - fn dummy() { - panic!("injected error (dap)"); - } }