Reduce the overridding surface area for integration test setup#1989
Reduce the overridding surface area for integration test setup#1989bidetofevil merged 2 commits intomainfrom
Conversation
bidetofevil
force-pushed
the
hho/reduce-test-setup-override
branch
3 times, most recently
from
b4c6263 to
4c61286
Compare
bidetofevil
force-pushed
the
hho/abandon-startup-onbackground
branch
from
95372a0 to
4fdf3f9
Compare
fractalwrench
left a comment
fractalwrench
left a comment
There was a problem hiding this comment.
LGTM, minor comments about fake workers no longer being required in some existing tests
| EmbraceSetupInterface(workerToFake = Worker.Background.LogMessageWorker).also { | ||
| executor = it.getFakedWorkerExecutor() | ||
| } |
There was a problem hiding this comment.
I'm not convinced this is required any more given the extensive refactors to the integration test framework that mean it's possible to block & wait until requests are delivered. I'd suggest deleting this and flushLogs entirely
There was a problem hiding this comment.
Let me try that. That would be great, even if we have to wait for the batch time to elapse, as that's more real.
There was a problem hiding this comment.
Yup! This seems to be the case. I had to make additional changes because it seems like faking the worker changes the order of the internal error that gets logged, so the tests were failing because it didn't find the right one.
I'll make a bigger change after merging this that makes it clear what I'm doing.
| EmbraceSetupInterface(workerToFake = Worker.Background.LogMessageWorker).also { | ||
| executor = it.getFakedWorkerExecutor() | ||
| } |
There was a problem hiding this comment.
Similar feedback to FlutterInternalInterfaceTest - I don't think fake workers are required anymore here
There was a problem hiding this comment.
That would be dope if true lol.
| ) | ||
| EmbraceSetupInterface(workerToFake = Worker.Background.LogMessageWorker).also { | ||
| executor = it.getFakedWorkerExecutor() | ||
| } |
There was a problem hiding this comment.
Similar feedback to FlutterInternalInterfaceTest - I don't think this is required anymore if we refactor the assertions a little bit
bidetofevil
changed the base branch from
hho/abandon-startup-onbackground
to
graphite-base/1989
bidetofevil
force-pushed
the
graphite-base/1989
branch
from
4fdf3f9 to
9044a8f
Compare
bidetofevil
force-pushed
the
hho/reduce-test-setup-override
branch
from
4c61286 to
7547b7c
Compare
bidetofevil
force-pushed
the
hho/reduce-test-setup-override
branch
from
7547b7c to
e39b3e6
Compare
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
2 participants
Goal
Simplify how you can use blocking executors for background and priority workers. This allows us to then not have to create new instances of
EmbFakeLoggerfor integration tests, thus allowing us to just set the internal error exclusion when instantiating an instance of that for the integration test rule. In turn, this allows us to get rid of the poorly named/documented factory method forFakeEmbLogger.