Skip to content

Minor fixes in testing and handling crlf correctly #405

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.

Sign up for GitHub

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

Open
nitinmotgi wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Minor fixes in testing and handling crlf correctly #405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion wrangler-core/src/main/java/io/cdap/wrangler/parser/MigrateToV2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public MigrateToV2(@Nullable String recipe, String delimiter) {
}

public MigrateToV2(String recipe) {
this(recipe, "\n");
this(recipe, "\\r?\\n");
Copy link
Contributor

@albertshau albertshau May 4, 2020

Choose a reason for hiding this comment

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

I've seen recipes with \r\n in the past and they seemed to work. Were only some directives stripping the \r or something?

}

/**
Expand Down
20 changes: 12 additions & 8 deletions wrangler-core/src/test/java/io/cdap/wrangler/parser/RecipeCompilerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,7 @@ public void testComplexExpression() throws Exception {
"send-to-error body_5 =~ \"DC.*\"",
"filter-rows-on regex-match body_5 *as*"
};
CompileStatus compile = TestingRig.compile(recipe);
Assert.assertTrue(true);
TestingRig.compileSuccess(recipe);
}

@Test
Expand All @@ -183,26 +182,31 @@ public void test() throws Exception {
"send-to-error body_5 =~ \"DC.*\"",
"filter-rows-on regex-match body_5 *as*"
};
CompileStatus compile = TestingRig.compile(recipe);
Assert.assertTrue(true);
TestingRig.compileSuccess(recipe);
}

@Test
public void testSingleLineDirectives() throws Exception {
String[] recipe = new String[] {
"parse-as-csv :body '\t' true; drop :body;"
};
CompileStatus compile = TestingRig.compile(recipe);
Assert.assertTrue(true);
TestingRig.compileSuccess(recipe);
}

@Test
public void testHandlingCRLF() throws Exception {
String[] recipe = new String[] {
"parse-as-csv :body '\t' true;\r\ndrop :body;\r\n"
};
TestingRig.compileSuccess(recipe);
Copy link
Contributor

@albertshau albertshau May 4, 2020

Choose a reason for hiding this comment

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

shouldn't just test that it compiles (didn't it compile successfully before?) Should test that the directives are parsed into the exact strings that are expected.

}

@Test
public void testError() throws Exception {
String[] recipe = new String[] {
"parse-as-abababa-csv :body '\t' true; drop :body;"
};
CompileStatus compile = TestingRig.compile(recipe);
Assert.assertTrue(true);
TestingRig.compileSuccess(recipe);
}

@Test
Expand Down