WyRainBow commented on code in PR #996:
URL:
https://github.com/apache/incubator-seata-go/pull/996#discussion_r2554726704
##########
pkg/datasource/sql/datasource/base/meta_cache.go:
##########
@@ -127,34 +124,43 @@ func (c *BaseTableMetaCache) refresh(ctx context.Context)
{
f()
- ticker := time.NewTicker(time.Duration(1 * time.Minute))
+ ticker := time.NewTicker(c.refreshInterval)
Review Comment:
Adding refreshInterval is mainly to improve test coverage and test
efficiency.
Test efficiency:
If the interval is hard-coded to 1 minute, each test run would take far too
long, which is impractical.
Test coverage:
If we don’t wait long enough, we can’t cover the logic that runs after
ticker.C fires (i.e., the second execution of f()). To reach the case
<-ticker.C: branch, the ticker must trigger quickly.
Therefore, exposing this field allows tests to control the interval, making
it possible to exercise that branch and improve overall coverage.
In unit tests (like TestBaseTableMetaCache_refresh
and TestBaseTableMetaCache_GracefulShutdown ) we need to verify the
behavior of the scheduled task.
If refreshInterval is hardcoded as a constant (eg 1 * time.Minute) inside
the refresh method, the test would have to wait for 1 minute to trigger the
logic, making the tests extremely slow.*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
