Skip to content
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

Fixture twins #85

Merged
merged 7 commits into from
Oct 25, 2017
Merged

Fixture twins #85

merged 7 commits into from
Oct 25, 2017

Conversation

rodionovsasha
Copy link
Owner

No description provided.

private boolean isYml(Path path) {
return Stream
.of(YAML_EXT, YML_EXT)
.anyMatch(ext -> getFileName(path).endsWith(ext));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

<Match>
<Class name="com.github.vkorobkov.jfixtures.util.StringUtil"/>
<Bug pattern="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE"/>
</Match>
<Match>
<Class name="com.github.vkorobkov.jfixtures.loader.FixturesLoader"/>
<Bug pattern="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this rule global?

}

private Fixture loadFixture(Path file) {
String name = getFixtureName(file);
val name = getFixtureName(file);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change does not concern the feature, so I'd remove it. Besides that, we probably don't have to always use val. We can leave simple and short types as they are. val is good for hiding long types and types with generics

@@ -149,6 +149,44 @@ class FixturesLoaderTest extends Specification implements YamlVirtualFolder {
role.columns.version == new FixtureValue(1)
}

def "should use yaml config"() {
Copy link
Collaborator

@vkorobkov vkorobkov Oct 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why "config"? What about "#load consumes fixture with .yaml extension"?

@vkorobkov
Copy link
Collaborator

@rodionovsasha this is the last part of the future - should update version in pom.xml?

String separator = file.getFileSystem().getSeparator();
String relativePath = Paths.get(path).relativize(file).toString();
relativePath = relativePath.substring(0, relativePath.lastIndexOf(".yml"));
val separator = file.getFileSystem().getSeparator();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep using types instead of val when types are short and don't have generics?

dima.columns.age == new FixtureValue(28)
}

def "load throws when config exists with yaml and yml extensions"() {
Copy link
Collaborator

@vkorobkov vkorobkov Oct 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"#load throws when a yaml fixture has a twin"

e.message == "Fixture exists with both extensions(yaml/yml)."
}

def "load throws when config exists with yml and yaml extensions"() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is redundant since the test above does the same. The order(yaml+yml VS yml+yaml) does not matter in this place, probably

<Bug pattern="DM_CONVERT_CASE"/>
</Match>
<Match>
<Class name="com.github.vkorobkov.jfixtures.config.structure.tables.CleanMethod"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Collaborator

@vkorobkov vkorobkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍰

@rodionovsasha rodionovsasha merged commit 573a15c into develop Oct 25, 2017
@rodionovsasha rodionovsasha deleted the fixtures-twins branch October 25, 2017 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants