Allow inject-able Singletons for test #1808
Assignees
Labels
Comments
|
It would be nice to do something similar for the static factory registries as well. |
htuch
pushed a commit
that referenced
this issue
Nov 20, 2017
Description: -Adds a ThreadSafe singleton class which can be accessed from all threads and overridden with an injected instance for custom implementations for test code or custom Envoy builds. -Replaces OsSysCallsImpl being plumbed everywhere it is needed with a new ThreadSafe singleton as a sample use case (and to avoid having to plumb the OsSysCallsImpl to all users for test overrides) -Adds abseil as a dependency Fixes #1808 Step 1 of #1754 Risk Level: Low: mainly a refactor to how OsSysCallsImpl is accessed Testing: Existing OS tests continue to pass, now using an inject-able mock singleton test/common/common/singleton_test.cc unit testing the new class Signed-off-by: Alyssa Wilk <[email protected]>
rshriram
pushed a commit
to rshriram/envoy
that referenced
this issue
Oct 30, 2018
jpsim
pushed a commit
that referenced
this issue
Nov 28, 2022
It turns out that removing Kotlin from Envoy Mobile is not really needed at this point. In reality, the Kotlin classes are a layer on top of the Java classes, and Cronvoy is a layer too: one level of indirection for the callbacks is enough. So instead Cronvoy uses the java classes directly. The main drawback is the CronvoyConfiguration - there is some duplication. Description: Have Cronvoy to only use the Envoy Mobile Java classes for its implementation Risk Level: None Testing: CI Docs Changes: N/A Release Notes: N/A Fixes #1658 Signed-off-by: Charles Le Borgne <[email protected]> Signed-off-by: JP Simard <[email protected]>
jpsim
pushed a commit
that referenced
this issue
Nov 29, 2022
It turns out that removing Kotlin from Envoy Mobile is not really needed at this point. In reality, the Kotlin classes are a layer on top of the Java classes, and Cronvoy is a layer too: one level of indirection for the callbacks is enough. So instead Cronvoy uses the java classes directly. The main drawback is the CronvoyConfiguration - there is some duplication. Description: Have Cronvoy to only use the Envoy Mobile Java classes for its implementation Risk Level: None Testing: CI Docs Changes: N/A Release Notes: N/A Fixes #1658 Signed-off-by: Charles Le Borgne <[email protected]> Signed-off-by: JP Simard <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
As disused briefly in #1802 if we need to mock out more system calls it'd probably be worthwhile instead of plumbing the OsSysCallsImpl through all relevant class hierarchies to have it be a singleton and have the Envoy Singleton class allow injection. Then for tests we could replace the static instance with a test-only instance which could do custom things like mock out bind calls and it would also allow testing corner cases with non-fatal syscall failure.
The text was updated successfully, but these errors were encountered: