Skip to content

[improve][ci] Run the integrationTest source set in CI#74

Merged
david-streamlio merged 1 commit into
apache:masterfrom
david-streamlio:ci/run-integration-tests
Jul 9, 2026
Merged

[improve][ci] Run the integrationTest source set in CI#74
david-streamlio merged 1 commit into
apache:masterfrom
david-streamlio:ci/run-integration-tests

Conversation

@david-streamlio

Copy link
Copy Markdown
Contributor

Motivation

The mqtt module ships a real Testcontainers integration test — MqttSinkIntegrationTest, added alongside the connector in #22 — in a separate integrationTest source set, with its own integrationTest Gradle task.

The CI workflow's tests matrix only runs test:

matrix:
  include:
    - name: Connectors
      tasks: test

So that integration test has never run on a pull request. The coverage exists but is not enforced: a change that broke MQTT delivery would go green.

This was found while auditing connectors for missing integration tests (#55). Every other module keeps its container tests in src/test, where CI picks them up; mqtt is the sole module with a separate source set.

Modifications

Add integrationTest to the matrix tasks. Verified that the task resolves repo-wide — ./gradlew integrationTest --dry-run succeeds, currently selecting only :mqtt:integrationTest.

Keeping the source set (rather than moving the test into src/test) preserves whatever intent led to the split, and leaves room for other modules to separate slow container tests from fast unit tests later.

Verifying this change

./gradlew :mqtt:integrationTest
org.apache.pulsar.io.mqtt.MqttSinkIntegrationTest > testWriteE2EWithMosquitto PASSED
BUILD SUCCESSFUL in 18s

The test was also mutation-checked: publishing to a different topic than the subscriber listens on makes it fail, so it genuinely guards delivery rather than passing against a broken target.

Note

This adds container-based work to the per-PR job. :mqtt:integrationTest takes ~18s locally, so the impact is small, but it is worth being explicit that the Tests - Connectors job now starts a Mosquitto container.

The mqtt module ships a Testcontainers integration test in a separate
integrationTest source set, added alongside the connector in apache#22. The
CI workflow only invokes the test task, so that test has never run on
a pull request and its coverage was not enforced.

Add integrationTest to the tests matrix. The task currently resolves to
mqtt only; other modules keep their container tests in src/test.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the GitHub Actions CI workflow so the repository’s existing integrationTest Gradle source set (notably :mqtt:integrationTest) is exercised on pull requests, closing a gap where container-based MQTT E2E coverage was present but not enforced.

Changes:

  • Extend the CI tests matrix for the “Connectors” job to run integrationTest in addition to test.
  • Ensure CI failure artifacts collection continues to capture test reports (including integration test reports).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@david-streamlio david-streamlio merged commit 47ed4c0 into apache:master Jul 9, 2026
2 checks passed
@david-streamlio david-streamlio deleted the ci/run-integration-tests branch July 9, 2026 22:16
david-streamlio added a commit that referenced this pull request Jul 10, 2026
…not hang CI (#91)

The Mosquitto GenericContainer had no wait strategy and no startup
timeout, so a stalled image pull hung @BeforeClass with no bound and
took the whole Tests - Connectors job to its 45-minute limit — a
cancellation rather than a failure, with no diagnostics. Observed on
run 29100595779.

Add Wait.forListeningPort() with a 3-minute startup timeout, plus a
test-level timeOut backstop. An unsatisfiable startup now fails in
about 20s with ContainerLaunchException instead of hanging.

This test only began running in CI when #74 added integrationTest to
the matrix, so the hazard was newly live.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants