ingest/ledgerbackend: Refactor stellarCoreRunner #4480
Conversation
… into stellarcorerunner-refactoring
| // We also want to use random directories in offline mode (reingestion) | ||
| // because it's possible it's running multiple Stellar-Cores on a single | ||
| // machine. | ||
| return path.Join(r.storagePath, "captive-core-"+createRandomHexString(8)) |
There was a problem hiding this comment.
are the comments, ln 196-204, still relevant for windows o/s, it looks like this would prevent windows o/s deployments from re-using the same storage path in runFrom correct?
There was a problem hiding this comment.
Yes, Windows is unable to shutdown internal process gracefully so in most cases sqlite DB is just broken after shutdown.
There was a problem hiding this comment.
ah, ok, should the entry for this change be added to Changelog.MD now, to mention this additional aspect that windows is not supported o/s for this optimization of re-using core's state db?
| // removeAll not called | ||
| runner.systemCaller = scMock | ||
|
|
||
| assert.NoError(t, runner.runFrom(100, "hash")) | ||
| assert.NoError(t, runner.close()) |
There was a problem hiding this comment.
maybe assert on storage path persistence after close, assert.DirExists(t, runner.StoragePath)
There was a problem hiding this comment.
Not sure if we should do that because after calling close the stellarCoreRunner instance shouldn't be reused. So we don't care about it's internal state anymore.
There was a problem hiding this comment.
the intent is to assert core's stellar.dbwhich resides on disk's runner.StoragePath is not deleted after shutdown of runner correct?
There was a problem hiding this comment.
OK, I misread the assertion as checking if runner.StoragePath is empty. Anyway, we don't need to do that because we ensure that mocked systemCaller.removeAll is not called. assert.DirExists checks if there's a real file in real filesystem. All of the filesystem calls in this test are mocked so no files are created during the test.
|
@sreuland PTAL. |
PR Checklist
PR Structure
otherwise).
services/friendbot, orallordocif the changes are broad or impact manypackages.
Thoroughness
.mdfiles, etc... affected by this change). Take a look in the
docsfolder for a given service,like this one.
Release planning
needed with deprecations, added features, breaking changes, and DB schema changes.
semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.
What
This commit refactors
stellarCoreRunnerto allow writing unit tests that does not call fs/os functions and adds unit tests for new functionality added in 4850d22. It also changesnewStellarCoreRunnerby removingmodeargument. The value will be set internally based on method calls.Why
Code and test infrastructure improvements.
Known limitations
[TODO or N/A]