core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider: start log poller in tests #11336
Conversation
|
I see that you haven't updated any CHANGELOG files. Would it make sense to do so? |
| @@ -660,6 +653,7 @@ func setupDependencies(t *testing.T, db *sqlx.DB, backend *backends.SimulatedBac | |||
| pollerLggr.SetLogLevel(zapcore.WarnLevel) | |||
| lorm := logpoller.NewORM(big.NewInt(1337), db, pollerLggr, pgtest.NewQConfig(false)) | |||
| lp := logpoller.NewLogPoller(lorm, ethClient, pollerLggr, 100*time.Millisecond, false, 1, 2, 2, 1000) | |||
| require.NoError(t, lp.Start(testutils.Context(t))) | |||
There was a problem hiding this comment.
None of the tests calling this setupDependencies helper had a running log poller. Was that intentional?
| @@ -534,7 +527,7 @@ func collectPayloads(ctx context.Context, t *testing.T, logProvider logprovider. | |||
| for ctx.Err() == nil && len(allPayloads) < n && rounds > 0 { | |||
| logs, err := logProvider.GetLatestPayloads(ctx) | |||
| require.NoError(t, err) | |||
| require.LessOrEqual(t, len(logs), logprovider.AllowedLogsPerUpkeep, "failed to get all logs") | |||
| require.LessOrEqual(t, len(logs), logprovider.AllowedLogsPerUpkeep) | |||
There was a problem hiding this comment.
This is where TestIntegration_LogEventProvider fails. Is this a per-round assumption? Meaning if this loop went fast enough, then the test would pass?
| if err := logProvider.Start(ctx); err != nil { | ||
| t.Logf("error starting log provider: %s", err) | ||
| t.Fail() |
There was a problem hiding this comment.
These don't need to be called from separate goroutines, and it was actually a problem since the call to Fail could race with the end of the test.
jmank88
force-pushed
the
logpoller-ctx
branch
3 times, most recently
from
2cef51d to
d0a87f4
Compare
jmank88
force-pushed
the
logpoller-ctx
branch
2 times, most recently
from
821bb80 to
3713078
Compare
|
SonarQube Quality Gate
|
|
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
jmank88
force-pushed
the
logpoller-ctx
branch
from
3713078 to
8fa0734
Compare
jmank88
changed the title
|
SonarQube Quality Gate
|
|
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
Another PR exposed that
TestIntegration_LogEventProviderwas not starting its log poller instance. However, with the log poller actuallyStarted and running,TestIntegration_LogEventProviderdoes not pass. It seems like there is a timing assumption baked in. Should it pass? Or does it depend on the log poller not running? If so, that is a bit unusual, since we don't normally interact with unstarted services.