tanclary commented on code in PR #2750:
URL: https://github.com/apache/calcite/pull/2750#discussion_r1437280790
##########
core/src/test/java/org/apache/calcite/util/SourceTest.java:
##########
@@ -121,6 +131,54 @@ void testAbsoluteFileToUrl(String path, String
expectedUrl) throws URISyntaxExce
() -> "Sources.of(Sources.of(file(" + path +
").absolutePath).url()).file().getPath()");
}
+ /**
+ * Resources created with the JAR protocol should also work.
Review Comment:
nit could reference JIRA here and in below test.
##########
core/src/test/java/org/apache/calcite/util/SourceTest.java:
##########
@@ -121,6 +131,54 @@ void testAbsoluteFileToUrl(String path, String
expectedUrl) throws URISyntaxExce
() -> "Sources.of(Sources.of(file(" + path +
").absolutePath).url()).file().getPath()");
}
+ /**
+ * Resources created with the JAR protocol should also work.
+ * This will enable the Fixture test to work under Bazel.
+ */
+ @Test void testJarFileUrl() throws MalformedURLException {
+ // mock jar file
+ final String jarPath = "jar:file:sources!/abcdef.txt";
+ final URL url = new URL(jarPath);
+ final Source source = of(url);
+ assertThat("No file retrieved for Sources.of(file " + jarPath + ")",
+ source.file(), notNullValue());
+ assertThat("Sources.of(file " + jarPath + ").url()).file().getPath()",
+ slashify(source.file().getPath()),
+ is("sources!/abcdef.txt"));
+
+ }
+
+ /** Tests {@link Sources#of(URL)} with code similar to
+ * {@code DiffRepository.Key#toRepo()}. */
+ @Test void testJarFileUrlWrite() {
+ final URL refFile = FoodmartQuerySet.class.getResource("/queries.json");
Review Comment:
To be honest I'm a bit confused at what all is going on here. Would you mind
either:
- Extracting some of the setup into a helper (maybe easier testing is
possible since this PR was opened).
- Breaking this into multiple tests
- Adding more comments
If it's all common sense and I'm OOTL then disregard.
##########
testkit/src/main/java/org/apache/calcite/test/DiffRepository.java:
##########
@@ -895,7 +898,17 @@ private static class Key {
DiffRepository toRepo() {
final URL refFile = findFile(clazz, ".xml");
final String refFilePath = Sources.of(refFile).file().getAbsolutePath();
- final String logFilePath = refFilePath.replace(".xml", "_actual.xml");
+ final String logFilePath;
+ if (StringUtils.containsIgnoreCase(refFilePath, ".jar!")) {
Review Comment:
I notice this block is identical to the one above, can we put it somewhere
helpful and refactor
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
