search

tinkerbell / hegel

Instance Metadata Service
https://tinkerbell.org
Apache License 2.0
99 stars 33 forks source link

HTTP tests use a hardcoded port which makes them brittle #73

Closed nshalman closed 2 years ago

nshalman commented 2 years ago

See https://github.com/tinkerbell/hegel/pull/72#issuecomment-998883680 (copied here)

The issue is the use of a hardcoded port https://github.com/tinkerbell/hegel/blob/9bb4194933c2b001b4edc461f27d04373690377b/http-server/http_server_test.go#L914-L915

being assigned to a global variable https://github.com/tinkerbell/hegel/blob/9bb4194933c2b001b4edc461f27d04373690377b/http-server/http_server.go#L26

This is exacerbated by the fact that during CI we run tests twice: https://github.com/tinkerbell/hegel/blob/9bb4194933c2b001b4edc461f27d04373690377b/.github/workflows/ci.yaml#L24-L27 This greatly increases the chances that the port won't be properly available. For now the easy fix is to consolidate to a single test run in CI, and later a deeper refactor can make this code more flexible about how it chooses a port for running tests.

chrisdoherty4 commented 2 years ago

86 and #87 go some way to improving this. The result of these PRs is centralized configuration and all dependencies injected so no more global state.

More to come.

chrisdoherty4 commented 2 years ago

Many tests are hard to follow and rely on implementation detail. Moreover, many are relying on actual HTTP endpoints being served which is unnecessary for mux endpoint configuration tests.

In conjunction with more holistic refactoring, we'll re-write many of the tests removing any reliance on implementation detail. As such, this issue can be closed as it'll be addressed with the re-write.