Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: Don't panic with no providers in client retrieve #9232

Merged
merged 1 commit into from
Aug 29, 2022

Conversation

magik6k
Copy link
Contributor

@magik6k magik6k commented Aug 29, 2022

Should address some gateway test flakiness like https://app.circleci.com/pipelines/github/filecoin-project/lotus/22805/workflows/2907bba9-2f9b-420b-9309-1078350bb9bb/jobs/564691

--- FAIL: TestGatewayCLIDealFlow (5.40s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1a941e2]

goroutine 25183 [running]:
testing.tRunner.func1.2({0x3a279a0, 0x5eebe00})
	/usr/local/go/src/testing/testing.go:1389 +0x24e
testing.tRunner.func1()
	/usr/local/go/src/testing/testing.go:1392 +0x39f
panic({0x3a279a0, 0x5eebe00})
	/usr/local/go/src/runtime/panic.go:838 +0x207
github.com/filecoin-project/lotus/cli.glob..func41(0xc0024542c0)
	/home/circleci/project/cli/client_retr.go:323 +0x502
github.com/filecoin-project/lotus/itests/kit.(*MockCLIClient).RunCmdRaw(0xc003497e80, {0xc00d7f7dc0?, 0x7f4dd406f0d0?, 0x38?})
	/home/circleci/project/itests/kit/mockcli.go:111 +0x323
github.com/filecoin-project/lotus/itests/kit.(*MockCLIClient).RunCmd(0xc003497e80, {0xc00d7f7dc0?, 0x0?, 0x0?})
	/home/circleci/project/itests/kit/mockcli.go:62 +0x2d
github.com/filecoin-project/lotus/itests/kit.RunClientTest(0x4545118?, {0x5f0a3c0, 0x12, 0x12}, 0xc0145e5c20)
	/home/circleci/project/itests/kit/client.go:119 +0x121d
command-line-arguments.TestGatewayCLIDealFlow(0xc010b922f0?)
	/home/circleci/project/itests/gateway_test.go:234 +0x6f
testing.tRunner(0xc014534820, 0x4244600)
	/usr/local/go/src/testing/testing.go:1439 +0x102
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:1486 +0x35f

@magik6k magik6k added the impact/test-flakiness Impact: Test Flakiness label Aug 29, 2022
@magik6k magik6k requested a review from a team as a code owner August 29, 2022 14:56
Copy link
Contributor

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

It's not clear to me how this fixes flakiness (I assume any test in q here will fail in this case even after this fix), but this is clearly a correcter thing to do.

@magik6k
Copy link
Contributor Author

magik6k commented Aug 29, 2022

This helps because the retrieval test calls the command in a loop, expecting it to error before the deal gets into a sector - https://github.com/filecoin-project/lotus/blob/master/itests/kit/client.go#L117-L124.

The problem was that if the deal didn't get into a sector before the first check (which it usually did, but not always - which is why this test was flaky, not always red), things would panic when we tried to deference the 'not-found' nil ExportRef.

@magik6k magik6k merged commit 2532300 into master Aug 29, 2022
@magik6k magik6k deleted the fix/gatewaytest-panic branch August 29, 2022 15:27
@arajasek
Copy link
Contributor

That makes sense, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/test-flakiness Impact: Test Flakiness
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants