Skip to content

KNOX-3482: Expand Knox health endpoint integration coverage - #1426

Open
Raghav-Mah3shwari wants to merge 1 commit into
apache:masterfrom
Raghav-Mah3shwari:KNOX-3482
Open

Raghav-Mah3shwari wants to merge 1 commit into
apache:masterfrom
Raghav-Mah3shwari:KNOX-3482

Conversation

@Raghav-Mah3shwari

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Added integration coverage for Knox health endpoints:

  • Verify POST /gateway/health/v1/ping.
  • Verify formatted metrics using ?pretty=true.
  • Verify gateway readiness states (OK or PENDING).
  • Verify required response headers.
  • Verify unknown health endpoint paths return 404.

How was this patch tested?

The Maven build completed successfully.

The Docker Compose integration test suite was rebuilt and executed using:

mvn clean -Ppackage,release install -T 1C
-Dmaven.test.skip=true -Dpmd.skip=true -Dcpd.skip=true
-Dcheckstyle.skip=true -Dspotbugs.skip=true -Drat.skip=true
-Dforbiddenapis.skip=true -Denforcer.skip=true -Djacoco.skip=true
-Dmaven.javadoc.skip=true -Dmaven.source.skip=true
-Dshellcheck.skip=true -Dxml.skip=true
-s .github/workflows/build/settings.xml

docker compose -f ./.github/workflows/compose/docker-compose.yml
build --no-cache knox-dev

docker compose -f ./.github/workflows/compose/docker-compose.yml
down --remove-orphans

docker compose -f ./.github/workflows/compose/docker-compose.yml
up --exit-code-from tests tests

Results:

•Maven build: BUILD SUCCESS
•Integration tests: 106 passed, 0 failed
•Pylint: 10.00/10

Integration Tests

Updated .github/workflows/tests/test_health.py. The tests are automatically discovered and executed by the Docker Compose test job configured through .github/workflows/tests.

UI changes

No UI changes.

@github-actions

Copy link
Copy Markdown

Test Results

  4 files    4 suites   42s ⏱️
120 tests 120 ✅ 0 💤 0 ❌
138 runs  138 ✅ 0 💤 0 ❌

Results for commit 900f0e4.

@hanicz hanicz 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.

This file already have 4 different /ping tests that could be improved imo. I feel like they test the same thing just separately. I think they could be unified since these are not unit tests.

msg="pretty metrics should include formatting whitespace",
)
self.assertIn("\n", pretty.text)
self.assertEqual(json.loads(compact.text), json.loads(pretty.text))

@hanicz hanicz Sep 24, 2026 •

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.

I don't like this, this can easily fail in the future due to flakiness. You compare two different live metric snapshots.

This branch has not been deployed

No deployments
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.

3 participants