From ee1de09a9e31a945cf8d931f104ccfdab9497372 Mon Sep 17 00:00:00 2001 From: Alex Petric Date: Mon, 21 Sep 2026 12:59:48 -0400 Subject: [PATCH] test: gate the sqs e2e test on its feature and pin the ee ref Review findings: without `sqs_trigger` the test built and ran against a server with no listener, so it could only time out; the aws-sdk-sqs dev dependency comes from `private`, which is why the old `#[ignore]` hid it. Also bump ee-repo-ref.txt so CI compiles the EE companion branch (the unit tests live there), drop a redundant set_var that raced other test threads, and stop the drain loop defaulting absent attributes to zero, which let the ack assertion pass on a broken response. Co-Authored-By: Claude Opus 5 (1M context) --- backend/ee-repo-ref.txt | 2 +- .../tests/trigger_e2e.rs | 34 ++++++++----------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index c336c4fdd3..53ba22c1bd 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -5370ae3a95a3dc10171f72da74286a654996eee6 +38699f705abb4e223d86e18b30d004aeaf589401 diff --git a/backend/windmill-api-integration-tests/tests/trigger_e2e.rs b/backend/windmill-api-integration-tests/tests/trigger_e2e.rs index 75dd53b346..8ce5ee3a5d 100644 --- a/backend/windmill-api-integration-tests/tests/trigger_e2e.rs +++ b/backend/windmill-api-integration-tests/tests/trigger_e2e.rs @@ -1,16 +1,10 @@ /*! * End-to-end integration tests for Windmill trigger listeners. * - * Each test is `#[ignore]` because it requires a running external service - * (MQTT broker, NATS server, Kafka broker, etc.). See individual test doc - * comments for setup instructions. - * - * Quick start — use the helper scripts in `tests/fixtures/`: - * ```bash - * ./tests/fixtures/start_all_triggers.sh # start all services - * ./tests/fixtures/start_all_triggers.sh oss # start OSS services only - * ./tests/fixtures/start_all_triggers.sh stop # tear down everything - * ``` + * Most tests are `#[ignore]` because they need an external service (MQTT broker, + * NATS server, Kafka broker, ...) that nothing starts for them. `test_sqs_e2e` is + * the exception: CI runs a LocalStack service for it, so it is not ignored. See + * each test's doc comment for how to run it locally. * * The general pattern: * 1. Insert a test script + trigger row + resource into the DB @@ -720,16 +714,16 @@ async fn test_nats_e2e(db: Pool) -> anyhow::Result<()> { /// cargo test --test trigger_e2e test_sqs_e2e \ /// --features sqs_trigger,enterprise,private -- --nocapture /// ``` -#[cfg(all(feature = "enterprise", feature = "private"))] +// `sqs_trigger` is what makes run_server spawn the SQS listener; the aws-sdk-sqs dev +// dependency arrives with `private`, so without this gate the test would build, run +// against a server that has no listener, and time out. +#[cfg(all(feature = "sqs_trigger", feature = "enterprise", feature = "private"))] #[sqlx::test(migrations = "../migrations", fixtures("base"))] async fn test_sqs_e2e(db: Pool) -> anyhow::Result<()> { use aws_sdk_sqs::types::QueueAttributeName; initialize_tracing().await; - // The SQS listener uses aws_config which respects AWS_ENDPOINT_URL for LocalStack. - std::env::set_var("AWS_ENDPOINT_URL", "http://localhost:4566"); - let script_path = "f/test/sqs_e2e_handler"; insert_test_script(&db, script_path).await?; @@ -788,7 +782,8 @@ async fn test_sqs_e2e(db: Pool) -> anyhow::Result<()> { .await?; let job = poll_for_trigger_job(&db, script_path, "sqs", Duration::from_secs(30)).await?; - // The handler script has no preprocessor, so the args are exactly the v1 payload. + // No preprocessor, so the args are the bare payload map with no `wm_trigger` extra. + // (SQS resolves to the V2 format here, but its v1 and v2 payload fns are identical.) assert_eq!( job.args.as_deref(), Some(&json!({ "msg": "hello from sqs e2e test" })), @@ -807,14 +802,15 @@ async fn test_sqs_e2e(db: Pool) -> anyhow::Result<()> { .attribute_names(QueueAttributeName::ApproximateNumberOfMessagesNotVisible) .send() .await?; - let count = |name| { + // Both attributes are always returned when requested, so treating a missing or + // unparseable one as zero would let the assertion pass on a broken response. + let count = |name: QueueAttributeName| { attrs .attributes() .and_then(|a| a.get(&name)) - .map(String::as_str) - .unwrap_or("0") + .unwrap_or_else(|| panic!("SQS did not return {name:?}")) .parse::() - .unwrap_or(0) + .expect("SQS returned a non-numeric message count") }; let visible = count(QueueAttributeName::ApproximateNumberOfMessages); let in_flight = count(QueueAttributeName::ApproximateNumberOfMessagesNotVisible);