-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Polymorphic persistence Issue #70 #83
Conversation
# Conflicts: # deepsampler-provider/src/testFixtures/java/de/ppi/deepsampler/provider/common/SamplerAspectTest.java # deepsampler-provider/src/testFixtures/java/de/ppi/deepsampler/provider/common/TestService.java
|
||
// Realen Typen Dog | ||
// Declared Type Animal | ||
// Generic null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comments seem to be scratches, they should be removed
} | ||
} else { | ||
returnClass= sampledMethod.getMethod().getReturnType(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code shuld be reformatted. Missing spaces after = and too long lines.
Usage of string.format() with placeholders instead of string-concatenation using + would also be better. PersistenceException has a convenient constructor for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am unsure what do you mean by, that there is a convinient constructor for a concatination in the constuctor of the exception.
...er-persistence/src/main/java/de/ppi/deepsampler/persistence/api/PersistentSampleManager.java
Outdated
Show resolved
Hide resolved
|
||
import java.util.Map; | ||
|
||
public class PolymorphicPersistentBean extends DefaultPersistentBean{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some javaodc is needed to explain what this is and why we have built it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
javadoc added
import java.lang.reflect.ParameterizedType; | ||
import java.lang.reflect.Type; | ||
|
||
public class BeanConversion { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this class shouldn't be in this branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class deleted
@Test | ||
public void polymorphicSamplesCanBeRecordedAndLoaded() throws IOException { | ||
// Ensure, that no Samples from previous tests exists... | ||
Sampler.clear(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This aleady done by @beforeeach
final PersistentSampleManager source = PersistentSampler.source(JsonSourceManager.builder().buildWithFile(pathToFile)); | ||
source.record(); | ||
|
||
// Reset SampleRepository to ensure that only deserialized samples will be used in the next steps... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is copied code. there is a method for that:
Line 535 in cb98769
private void clearSampleRepositoryWithAssertion() { |
@@ -544,4 +544,40 @@ private PersistentSampleManager save(Path pathToFile) { | |||
return source; | |||
} | |||
|
|||
@Test | |||
public void polymorphicSamplesCanBeRecordedAndLoaded() throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The extension TempJsonFile injects a Path to a tmp file and deletes that file after the test has run. You can add a parameter Path tempFile
to the method to get access to the tmp file.
There is also a helper-method that uses the tmpFile to create a PersistnetSampleManager:
Line 541 in cb98769
private PersistentSampleManager save(Path pathToFile) { |
…rphic-feature # Conflicts: # README.md # deepsampler-persistence/src/main/java/de/ppi/deepsampler/persistence/api/PersistentSampleManager.java # deepsampler-provider/src/testFixtures/java/de/ppi/deepsampler/provider/common/Cat.java # deepsampler-provider/src/testFixtures/java/de/ppi/deepsampler/provider/common/PersistentSamplerAspectTest.java # deepsampler-provider/src/testFixtures/java/de/ppi/deepsampler/provider/common/TestService.java
…ension. The class is not used yet, and it messes up the test coverage
…ension. The class is not used yet, and it messes up the test coverage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
review should be completed
return (T) originalBean; | ||
} | ||
|
||
/** | ||
* Todo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todos resolved
import java.lang.reflect.ParameterizedType; | ||
import java.lang.reflect.Type; | ||
|
||
public class BeanConversion { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class deleted
|
||
import java.util.Map; | ||
|
||
public class PolymorphicPersistentBean extends DefaultPersistentBean{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
javadoc added
Kudos, SonarCloud Quality Gate passed! |
No description provided.