Skip to content

Conversation

@eriktate
Copy link
Contributor

@eriktate eriktate commented Sep 4, 2025

No description provided.

@eriktate eriktate added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v16 backport/branch/v17 backport/branch/v18 labels Sep 4, 2025
@eriktate eriktate changed the title Replace assert with require in hos tuser integration tests Replace assert with require in host user integration tests Sep 4, 2025
Copy link
Contributor

@rosstimothy rosstimothy left a comment

Choose a reason for hiding this comment

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

I think we should revert the mass conversion from assert to require in the spirit of keep-going. The one place we either need to use require, or return if an assert.NoError fails is in the EventuallyWithT blocks to prevent panics like

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x16e7d0f9]

goroutine 15065 [running]:
github.com/gravitational/teleport/integration.testStaticHostUsers.func2(0xc00428bad0)
	/__w/teleport/teleport/integration/hostuser_test.go:879 +0xd9
github.com/stretchr/testify/assert.EventuallyWithT.func1()
	/go/pkg/mod/github.com/stretchr/[email protected]/assert/assertions.go:2096 +0xb8
created by github.com/stretchr/testify/assert.EventuallyWithT in goroutine 5479
	/go/pkg/mod/github.com/stretchr/[email protected]/assert/assertions.go:2108 +0x27d

The above comment applied to the original commit which has since been resolved by the squashed commit.

@eriktate eriktate force-pushed the eriktate/fix-hostuser-assertions branch from 6efdba1 to f6bd24e Compare September 4, 2025 16:18
@rosstimothy rosstimothy requested a review from cthach September 4, 2025 17:24
Copy link
Contributor

@cthach cthach left a comment

Choose a reason for hiding this comment

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

LGTM

@rosstimothy rosstimothy added this pull request to the merge queue Sep 4, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 4, 2025
@eriktate eriktate added this pull request to the merge queue Sep 4, 2025
Merged via the queue into master with commit e36b7c0 Sep 4, 2025
40 checks passed
@eriktate eriktate deleted the eriktate/fix-hostuser-assertions branch September 4, 2025 19:02
@backport-bot-workflows
Copy link
Contributor

@eriktate See the table below for backport results.

Branch Result
branch/v16 Create PR
branch/v17 Create PR
branch/v18 Create PR

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

Labels

backport/branch/v17 backport/branch/v18 no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants