Skip to content

Conversation

@pdesoyres-cc
Copy link
Contributor

@pdesoyres-cc pdesoyres-cc commented Feb 3, 2026

Context

The e2e test suite creates applications with log drains attached during log command tests. After tests complete, the cleanup phase tears down all created resources (applications, addons, organisations, etc.).

Problem

Log drains created during tests were not being cleaned up because the API does not automatically delete them when the application is deleted. This left orphaned log drains behind after each test run.

Additionally, cleanup used Promise.all which would abort early on the first rejection, meaning subsequent resources were never cleaned up. Test timeouts were also too low for slow API responses, causing spurious failures.

The access log test asserted type as "http" but the API actually returns "https".

Solution

  • Delete log drains before applications: During cleanup, list and delete all log drains attached to an application before deleting the application itself.
  • Switch Promise.all to Promise.allSettled in all cleanup functions so that a single failure doesn't prevent other resources from being cleaned up.
  • Increase test timeouts from 10s to 30s (per-test) and from 10min to 20min (test suite finish) to accommodate slow API responses.
  • Fix access log type assertion from strict string assertion to just expect a string.

Pierre DE SOYRES added 3 commits February 3, 2026 09:59
When a resource is already gone or a deletion fails, Promise.all
short-circuits and skips remaining cleanup tasks. Using allSettled
ensures all resources are attempted for deletion independently.
API response times can vary significantly, causing e2e tests to fail
intermittently with the previous limits.
@pdesoyres-cc pdesoyres-cc force-pushed the e2e-log-drains-cleanup branch from 8a27a76 to 5ae7907 Compare February 3, 2026 10:12
Copy link
Member

@Galimede Galimede left a comment

Choose a reason for hiding this comment

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

Hey Pierre, just two quick questions but LGTM, thanks for handling that! 💪

);
if (drains.length > 0) {
await Promise.allSettled(
drains.map((drain) =>
Copy link
Member

Choose a reason for hiding this comment

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

question: We don't seem to use the array after the map, do we really need to use map?

},
},
testsFinishTimeout: 1000 * 60 * 10,
testsFinishTimeout: 1000 * 60 * 20,
Copy link
Member

Choose a reason for hiding this comment

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

question: what's the difference between timeout and testsFinishTimeout? Before it was all set to 10 seconds now we have both 20 and 30 seconds, but i'm not sure I understand what one or the other does

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants