Skip to content

Comments

Fix flaky test: k8s script v1 integration test#1189

Merged
todthomson merged 3 commits intomainfrom
eft/fix/flaky-test/k8s-script-v1-integration-test
Feb 23, 2026
Merged

Fix flaky test: k8s script v1 integration test#1189
todthomson merged 3 commits intomainfrom
eft/fix/flaky-test/k8s-script-v1-integration-test

Conversation

@todthomson
Copy link
Contributor

@todthomson todthomson commented Feb 23, 2026

Background

Because of an historically intermittent (flaky) test run failure, which triggers alerts to @OctopusDeploy/team-executions-foundations in test:

Octopus.Tentacle.Kubernetes.Tests.Integration
.KubernetesAgent.KubernetesScriptServiceV1IntegrationTest.RunSimpleScript()

caused by:

Server exception: 
System.IO.EndOfStreamException: Attempted to read past the end of the stream.
at Halibut.Transport.Protocol.MessageSerializer
.ReadMessageAsync[T](RewindableBufferStream stream, CancellationToken cancellationToken)

which results in:

An error occurred communicating with Tentacle.
This action will be retried after 1 seconds.
Retry attempt 1.
Retries will be performed for up to 89 seconds.

which subsequently results in an assertion failure:

Started to be 1L, but found 2L.

caused by the following assertion:

recordedMethodUsages.For(nameof(IAsyncClientKubernetesScriptServiceV1.StartScriptAsync))
.Started.Should().Be(1);
=> 2

but I think that's perfectly reasonable (i.e. it's reasonable to restart when loosing comms with Tentacle).

Results

Thus, I've updated the assertion to be "at least once":

recordedMethodUsages.For(nameof(IAsyncClientKubernetesScriptServiceV1.StartScriptAsync))
.Started.Should().BeGreaterThanOrEqualTo(1);

Before

Flaky test e.g.
https://build.octopushq.com/buildConfiguration/TeamFireAndMotion_OctopusTentacle_TentacleVLatest_net80_ChainFullChainNightly/21130454

After

No more flaky test.

How to review this PR

This is how Octopus behaves when the connection to Tentacle is interrupted.

Do you agree that, therefore, the assertion in the original test "as written" was wrong?

If so, please approve.

If not, please talk to me and explain why (the assertion should remain as it is)?

Pre-requisites

Yes, this is a flaky test, I don't think it needs a GitHub issue.

  • I have considered informing or consulting the right people, according to the ownership map.

Consultation plan:

  1. Talk internally within my team (to see if I'm on the right track).
  2. Talk to the original test author @APErebus (to confirm that I'm on the right track).
  • I have considered appropriate testing for my change.

My change IS a test, and, it will run in the CI environment.

because sometimes connection to Tentacle fails, and
Tentacle does the right thing (I think) and starts the script again.
@todthomson todthomson requested a review from a team February 23, 2026 02:26
@todthomson todthomson marked this pull request as ready for review February 23, 2026 02:34
@todthomson todthomson requested a review from a team as a code owner February 23, 2026 02:34
Copy link

@julietrb1 julietrb1 left a comment

Choose a reason for hiding this comment

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

I like it very much, but I've suggested shortening the comment and adding it to this PR's details instead.

@todthomson todthomson enabled auto-merge (rebase) February 23, 2026 02:38
Copy link
Contributor

@Jtango18 Jtango18 left a comment

Choose a reason for hiding this comment

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

Approved by MD - the logic behind it makes sense

@APErebus - if you find this abhorent, we can always reverse down the track.

@todthomson todthomson merged commit 2de14cd into main Feb 23, 2026
51 checks passed
@todthomson todthomson deleted the eft/fix/flaky-test/k8s-script-v1-integration-test branch February 23, 2026 03:48
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.

4 participants