Skip to content

Commit a51beff

Browse files
committed
fix(test): concurrent access to global OTEL configuration.
The OTEL test is flaky because it relies on the global OTEL configuration that can be overwrite by other concurrent tests. Therefore, the OTEL test now is behind a feature gate to ensure that it will be run on isolation. Signed-off-by: José Guilherme Vanz <jguilhermevanz@suse.com>
1 parent 772b974 commit a51beff

File tree

4 files changed

+51
-38
lines changed

4 files changed

+51
-38
lines changed

.github/workflows/ci.yml

+19-21
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,8 @@ jobs:
1919
profile: minimal
2020
toolchain: stable
2121
override: true
22-
- uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505 # v1.0.3
23-
with:
24-
command: check
22+
- name: Run cargo check
23+
run: cargo check
2524

2625
version-check:
2726
name: Check Cargo.toml version
@@ -50,10 +49,8 @@ jobs:
5049
profile: minimal
5150
toolchain: stable
5251
override: true
53-
- uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505 # v1.0.3
54-
with:
55-
command: test
56-
args: --workspace --lib
52+
- name: Run unit-tests coverage
53+
run: make unit-tests
5754

5855
integration-tests:
5956
name: Integration tests
@@ -65,10 +62,8 @@ jobs:
6562
profile: minimal
6663
toolchain: stable
6764
override: true
68-
- uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505 # v1.0.3
69-
with:
70-
command: test
71-
args: --test '*'
65+
- name: Run integration-tests coverage
66+
run: make integration-tests
7267

7368
fmt:
7469
name: Rustfmt
@@ -80,11 +75,8 @@ jobs:
8075
profile: minimal
8176
toolchain: stable
8277
override: true
83-
- run: rustup component add rustfmt
84-
- uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505 # v1.0.3
85-
with:
86-
command: fmt
87-
args: --all -- --check
78+
- name: Run Rust format check
79+
run: make fmt
8880

8981
clippy:
9082
name: Clippy
@@ -96,11 +88,8 @@ jobs:
9688
profile: minimal
9789
toolchain: stable
9890
override: true
99-
- run: rustup component add clippy
100-
- uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505 # v1.0.3
101-
with:
102-
command: clippy
103-
args: -- -D warnings
91+
- run: Run Clippy
92+
run: make lint
10493

10594
coverage:
10695
name: coverage
@@ -135,6 +124,15 @@ jobs:
135124
directory: coverage/integration-tests
136125
flags: integration-tests
137126
verbose: true
127+
- name: Upload OTEL integration-tests coverage to Codecov
128+
uses: codecov/codecov-action@13ce06bfc6bbe3ecf90edbbf1bc32fe5978ca1d3 # v5.3.1
129+
env:
130+
CODECOV_TOKEN: ${{ secrets.CODECOV_ORG_TOKEN }}
131+
with:
132+
name: otel-integration-tests
133+
directory: coverage/otel-integration-tests
134+
flags: integration-tests
135+
verbose: true
138136

139137
docs:
140138
name: Update documentation

Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -91,3 +91,6 @@ reqwest = { version = "0.12", default-features = false, features = [
9191
"http2",
9292
"rustls-tls-manual-roots",
9393
] }
94+
95+
[features]
96+
otel_tests = []

Makefile

+5-1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ unit-tests: fmt lint
3232
.PHONY: integration-test
3333
integration-tests: fmt lint
3434
cargo test --test '*'
35+
cargo test --features otel_tests -- test_otel
3536

3637
.PHONY: coverage
3738
coverage: coverage-unit-tests coverage-integration-tests
@@ -45,8 +46,11 @@ coverage-unit-tests:
4546
.PHONY: coverage-integration-tests
4647
coverage-integration-tests:
4748
cargo tarpaulin --verbose --skip-clean --engine=llvm \
48-
--all-features --implicit-test-threads --test integration_test \
49+
--implicit-test-threads --test integration_test \
4950
--out xml --out html --output-dir coverage/integration-tests
51+
cargo tarpaulin --verbose --skip-clean --engine=llvm \
52+
--features otel_tests --implicit-test-threads --test integration_test \
53+
--out xml --out html --output-dir coverage/otel-integration-tests -- test_otel
5054

5155
.PHONY: clean
5256
clean:

tests/integration_test.rs

+24-16
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
mod common;
22

3-
use std::io::BufReader;
3+
use std::path::PathBuf;
44
use std::{
55
collections::{BTreeSet, HashMap},
6-
fs::{set_permissions, File, Permissions},
7-
io::BufRead,
8-
os::unix::fs::PermissionsExt,
9-
path::PathBuf,
106
time::Duration,
117
};
8+
#[cfg(feature = "otel_tests")]
9+
use std::{fs::File, io::BufRead};
1210

1311
use common::{app, setup};
1412

@@ -26,18 +24,9 @@ use policy_evaluator::{
2624
use policy_server::{
2725
api::admission_review::AdmissionReviewResponse,
2826
config::{PolicyMode, PolicyOrPolicyGroup},
29-
metrics::setup_metrics,
30-
tracing::setup_tracing,
3127
};
32-
use rcgen::{BasicConstraints, CertificateParams, DnType, IsCa, KeyPair};
3328
use regex::Regex;
3429
use rstest::*;
35-
use tempfile::NamedTempFile;
36-
use testcontainers::{
37-
core::{Mount, WaitFor},
38-
runners::AsyncRunner,
39-
GenericImage, ImageExt,
40-
};
4130
use tokio::fs;
4231
use tower::ServiceExt;
4332

@@ -804,10 +793,23 @@ async fn test_detect_certificate_rotation() {
804793
}
805794
}
806795

807-
// This test is flaky, needs fix. Ignoring for release.
808-
#[ignore]
796+
// The OTEL test is behind a feature flag because it needs to ensure that the
797+
// global OTEL configuration is not overwritten by other concurrent tests.
809798
#[tokio::test]
799+
#[cfg(feature = "otel_tests")]
810800
async fn test_otel() {
801+
use policy_server::{metrics::setup_metrics, tracing::setup_tracing};
802+
use std::{
803+
fs::{set_permissions, Permissions},
804+
os::unix::fs::PermissionsExt,
805+
};
806+
use tempfile::NamedTempFile;
807+
use testcontainers::{
808+
core::{Mount, WaitFor},
809+
runners::AsyncRunner,
810+
GenericImage, ImageExt,
811+
};
812+
811813
setup();
812814

813815
let otelc_config_path =
@@ -970,9 +972,12 @@ async fn test_otel() {
970972
otelc.stop().await.unwrap();
971973
}
972974

975+
#[cfg(feature = "otel_tests")]
973976
async fn parse_exporter_output(
974977
exporter_output_file: &File,
975978
) -> serde_json::Result<serde_json::Value> {
979+
use std::io::BufReader;
980+
976981
let mut reader = BufReader::new(exporter_output_file);
977982

978983
// read only the first entry in the output file
@@ -984,7 +989,10 @@ async fn parse_exporter_output(
984989
serde_json::from_str(&exporter_output)
985990
}
986991

992+
#[cfg(feature = "otel_tests")]
987993
fn generate_tls_certs() -> (String, String, String) {
994+
use rcgen::{BasicConstraints, CertificateParams, DnType, IsCa, KeyPair};
995+
988996
let ca_key = KeyPair::generate().unwrap();
989997
let mut params = CertificateParams::new(vec!["My Test CA".to_string()]).unwrap();
990998
params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained);

0 commit comments

Comments
 (0)