Introduce StorageFactorySwitch
#289
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Current coverage is 86.08% (diff: 91.66%)@@ master #289 diff @@
==========================================
Files 191 192 +1
Lines 6572 6618 +46
Methods 0 0
Messages 0 0
Branches 641 649 +8
==========================================
+ Hits 5652 5697 +45
Misses 807 807
- Partials 113 114 +1
|
Also test for parameterless ctor in `StorageFactorySwitch`.
armiol
requested changes
Jan 27, 2017
| * a {@code Supplier} for tests was not set via {@link #init(Supplier, Supplier)}. | ||
| * | ||
| * <h2>Production mode</h2> | ||
| * <p>In production mode this class obtains the instance obtained |
There was a problem hiding this comment.
...obtains the instance obtained... looks weird.
I think obtains the instance provided by the will do the job.
| * <h2>Production mode</h2> | ||
| * <p>In production mode this class obtains the instance obtained | ||
| * by the {@code Supplier} passed via {@link #init(Supplier, Supplier)}. | ||
| * If the production {@code Supplier} was not initialized {@code IllegalStateException} |
There was a problem hiding this comment.
I think, a comma after the not initialized would improve the readability.
| @VisibleForTesting | ||
| static final String ENV_KEY_APP_ENGINE_RUNTIME_VERSION = "com.google.appengine.runtime.version"; | ||
|
|
||
| /** If set contains the version of AppEngine obtained from the system property. */ |
There was a problem hiding this comment.
Let's add a comma after If set.
| } | ||
|
|
||
| /** | ||
| * Turns the environment into tests mode. |
There was a problem hiding this comment.
Perhaps, Switches ...?
Or maybe Turns on the test mode for the environment.?
| } | ||
|
|
||
| /** | ||
| * Cleans the `storageFactorySwitch` instance by nullifying fields. |
There was a problem hiding this comment.
I think, clear will be more suitable as an action verb for the intention like this.
Another option to consider is restore, if you want to emphasise the returning to the original state.
|
|
||
| @Test | ||
| public void cache_instance_of_StorageFactory_in_testing() { | ||
| Supplier<StorageFactory> testingSupplier = spy(inMemorySupplier); |
|
|
||
| @Test | ||
| public void cache_instance_of_StorageFactory_in_production() { | ||
| Supplier<StorageFactory> productionSupplier = spy(inMemorySupplier); |
|
@armiol PTAL |
Also addressed missed `final` for test var.
armiol
approved these changes
Jan 27, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.