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.

Sign up for GitHub

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

Jump to bottom

core/capabilities/ccip/ccip_integration_tests/ccipreader: close logpoller in testSetup() #15335

Merged
merged 1 commit into from
Nov 20, 2024
Merged

core/capabilities/ccip/ccip_integration_tests/ccipreader: close logpoller in testSetup() #15335

merged 1 commit into from
Nov 20, 2024

Conversation

jmank88
Copy link
Contributor

@jmank88 jmank88 commented Nov 20, 2024

A log poller instance was left running after test completion.

==================
WARNING: DATA RACE
Read at 0x00c00191a383 by goroutine 430:
  testing.(*common).logDepth()
      /opt/hostedtoolcache/go/1.23.3/x64/src/testing/testing.go:1024 +0x504
  testing.(*common).log()
      /opt/hostedtoolcache/go/1.23.3/x64/src/testing/testing.go:1011 +0xa4
  testing.(*common).Logf()
      /opt/hostedtoolcache/go/1.23.3/x64/src/testing/testing.go:1062 +0x6a
  testing.(*T).Logf()
      <autogenerated>:1 +0x69
  go.uber.org/zap/zaptest.TestingWriter.Write()
      /home/runner/go/pkg/mod/go.uber.org/[email protected]/zaptest/logger.go:146 +0x11d
  go.uber.org/zap/zaptest.(*TestingWriter).Write()
      <autogenerated>:1 +0x74
  go.uber.org/zap/zapcore.(*ioCore).Write()
      /home/runner/go/pkg/mod/go.uber.org/[email protected]/zapcore/core.go:99 +0x192
  go.uber.org/zap/zapcore.(*CheckedEntry).Write()
      /home/runner/go/pkg/mod/go.uber.org/[email protected]/zapcore/entry.go:253 +0x1ef
  go.uber.org/zap.(*SugaredLogger).log()
      /home/runner/go/pkg/mod/go.uber.org/[email protected]/sugar.go:355 +0x12f
  go.uber.org/zap.(*SugaredLogger).Errorw()
      /home/runner/go/pkg/mod/go.uber.org/[email protected]/sugar.go:269 +0xa4
  github.com/smartcontractkit/chainlink/v2/core/logger.(*zapLogger).Errorw()
      <autogenerated>:1 +0x1f
  github.com/smartcontractkit/chainlink-common/pkg/logger.(*sugared).Errorw()
      <autogenerated>:1 +0x81
  github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller.(*logPoller).run()
      /home/runner/work/chainlink/chainlink/core/chains/evm/logpoller/log_poller.go:635 +0x792
  github.com/smartcontractkit/chainlink/v2/core/capabilities/ccip/ccip_integration_tests/ccipreader.testSetup.(*logPoller).Start.func3.gowrap1()
      /home/runner/work/chainlink/chainlink/core/chains/evm/logpoller/log_poller.go:508 +0x33

Previous write at 0x00c00191a383 by main goroutine:
  testing.tRunner.func1()
      /opt/hostedtoolcache/go/1.23.3/x64/src/testing/testing.go:1677 +0x8fa
  runtime.deferreturn()
      /opt/hostedtoolcache/go/1.23.3/x64/src/runtime/panic.go:605 +0x5d
  testing.runTests()
      /opt/hostedtoolcache/go/1.23.3/x64/src/testing/testing.go:2166 +0x8be
  testing.(*M).Run()
      /opt/hostedtoolcache/go/1.23.3/x64/src/testing/testing.go:2034 +0xf17
  main.main()
      _testmain.go:63 +0x164

Goroutine 430 (running) created at:
  github.com/smartcontractkit/chainlink/v2/core/capabilities/ccip/ccip_integration_tests/ccipreader.testSetup.(*logPoller).Start.func3()
      /home/runner/work/chainlink/chainlink/core/chains/evm/logpoller/log_poller.go:508 +0xad
  github.com/smartcontractkit/chainlink-common/pkg/services.(*StateMachine).StartOnce()
      /home/runner/go/pkg/mod/github.com/smartcontractkit/[email protected]/pkg/services/state.go:93 +0x112
  github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller.(*logPoller).Start()
      /home/runner/work/chainlink/chainlink/core/chains/evm/logpoller/log_poller.go:506 +0x1caf
  github.com/smartcontractkit/chainlink/v2/core/capabilities/ccip/ccip_integration_tests/ccipreader.testSetup()
      /home/runner/work/chainlink/chainlink/core/capabilities/ccip/ccip_integration_tests/ccipreader/ccipreader_test.go:857 +0x1cb0
  github.com/smartcontractkit/chainlink/v2/core/capabilities/ccip/ccip_integration_tests/ccipreader.Test_GetChainFeePriceUpdates()
      /home/runner/work/chainlink/chainlink/core/capabilities/ccip/ccip_integration_tests/ccipreader/ccipreader_test.go:511 +0x2ed
  testing.tRunner()
      /opt/hostedtoolcache/go/1.23.3/x64/src/testing/testing.go:1690 +0x226
  testing.(*T).Run.gowrap1()
      /opt/hostedtoolcache/go/1.23.3/x64/src/testing/testing.go:1743 +0x44
==================

@jmank88 jmank88 requested a review from a team as a code owner November 20, 2024 15:00
jmank88
jmank88 commented Nov 20, 2024
View reviewed changes
core/capabilities/ccip/ccip_integration_tests/ccipreader/ccipreader_test.go
@@ -797,6 +798,7 @@ func testSetup(
lggr := logger.TestLogger(t)
lggr.SetLogLevel(zapcore.ErrorLevel)
db := pgtest.NewSqlxDB(t)
t.Cleanup(func() { assert.NoError(t, db.Close()) })
Copy link
Contributor Author

@jmank88 jmank88 Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It is important to register defer/Cleanup calls immediately, no only so they are not forgotten (like lp2 was in this case), but because they must run after this point no matter how or when the test exits - which could be before the old Cleanup call at the end.

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @asoliman92

jmank88
jmank88 commented Nov 20, 2024
View reviewed changes
core/capabilities/ccip/ccip_integration_tests/ccipreader/ccipreader_test.go
@@ -812,7 +814,7 @@ func testSetup(
headTracker,
lpOpts,
)
assert.NoError(t, lp.Start(ctx))
servicetest.Run(t, lp)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For service.Services, this helper can be used to invoke Start and register Close automatically.

@jmank88 jmank88 enabled auto-merge November 20, 2024 15:04
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 20, 2024 •
edited
Loading

AER Report: CI Core ran successfully ✅

aer_workflow , commit

@jmank88 jmank88 mentioned this pull request Nov 20, 2024
Merged
@jmank88 jmank88 requested a review from 0xnogo November 20, 2024 16:14
@jmank88 jmank88 added this pull request to the merge queue Nov 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 20, 2024
@jmank88 jmank88 added this pull request to the merge queue Nov 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 20, 2024
@jmank88 jmank88 added this pull request to the merge queue Nov 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 20, 2024
@jmank88 jmank88 added this pull request to the merge queue Nov 20, 2024
Merged via the queue into develop with commit 2596452 Nov 20, 2024
165 checks passed
@jmank88 jmank88 deleted the ccipreader-test-race branch November 20, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krehermann krehermann krehermann approved these changes

@vyzaldysanchez vyzaldysanchez vyzaldysanchez approved these changes

@makramkd makramkd makramkd approved these changes

@samsondav samsondav samsondav approved these changes

@winder winder Awaiting requested review from winder winder is a code owner automatically assigned from smartcontractkit/ccip-offchain

@rstout rstout Awaiting requested review from rstout rstout is a code owner automatically assigned from smartcontractkit/ccip-offchain

@asoliman92 asoliman92 Awaiting requested review from asoliman92 asoliman92 is a code owner automatically assigned from smartcontractkit/ccip-offchain

@dimkouv dimkouv Awaiting requested review from dimkouv dimkouv is a code owner automatically assigned from smartcontractkit/ccip-offchain

@mateusz-sekara mateusz-sekara Awaiting requested review from mateusz-sekara mateusz-sekara is a code owner automatically assigned from smartcontractkit/ccip-offchain

@0xAustinWang 0xAustinWang Awaiting requested review from 0xAustinWang 0xAustinWang is a code owner automatically assigned from smartcontractkit/ccip-offchain

@0xnogo 0xnogo Awaiting requested review from 0xnogo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jmank88 @samsondav @makramkd @vyzaldysanchez @krehermann