[jira] [Commented] (CALCITE-5980) QuidemTests are not effectively executed on Windows
[ https://issues.apache.org/jira/browse/CALCITE-5980?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17762840#comment-17762840 ] Ruben Q L commented on CALCITE-5980: Sure. PR updated. > QuidemTests are not effectively executed on Windows > --- > > Key: CALCITE-5980 > URL: https://issues.apache.org/jira/browse/CALCITE-5980 > Project: Calcite > Issue Type: Bug > Components: core >Affects Versions: 1.35.0 >Reporter: Ruben Q L >Assignee: Ruben Q L >Priority: Major > Fix For: 1.36.0 > > > Discovered by accident on my Windows+IntelliJ environment while I was trying > to add new tests on a *.iq file. My new tests did not seem to be executed. I > even tried adding syntax errors on purpose into different iq files to force > them to fail, but the tests were still successful. The reason seems to be > that, at least on my environment (Windows), the test files do not execute any > of their statements. This seems caused by the changes introduced in > CALCITE-5786. > While debugging, I found this line in QuidemTest.java (that aims to create > the inFile and outFile): > {code:java} > protected void checkRun(String path) throws Exception { > ... > // e.g. path = "sql/agg.iq" > // inUrl = "file:/home/fred/calcite/core/build/resources/test/sql/agg.iq" > // inFile = "/home/fred/calcite/core/build/resources/test/sql/agg.iq" > // outDir = "/home/fred/calcite/core/build/quidem/test/sql" > // outFile = "/home/fred/calcite/core/build/quidem/test/sql/agg.iq" > inFile = Sources.of(requireNonNull(inUrl, "inUrl")).file(); > outFile = replaceDir(inFile, "resources", "quidem"); > ... > } > {code} > But in my case it results on {*}both files being the same{*}, thus when the > outFile is created, it actually erases all the tests that were contained > inside the inFile, so the test runs nothing. > The reason for that is that the auxiliary method {{{}replaceDir{}}}: > {code:java} > private static File replaceDir(File file, String target, String > replacement) { > return new File( > file.getAbsolutePath().replace(n2u('/' + target + '/'), > n2u('/' + replacement + '/'))); > } > {code} > is trying to replace "/resources/" with "/quidem/" from the inFile absolute > path, but in my case this path does not contain the pattern to be replaced, > since it contains backslashes: > "C:\...\calcite\core\build\resources\test\sql\agg.iq"; so the replacement > operation does nothing. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (CALCITE-5980) QuidemTests are not effectively executed on Windows
[ https://issues.apache.org/jira/browse/CALCITE-5980?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17762824#comment-17762824 ] Julian Hyde commented on CALCITE-5980: -- That makes sense. Can you add a comment that we disallow empty inFile because it is a likely indication of overwrite? It's not technically wrong to have an empty inFile (just as utilities such as {{cat}} and {{wc}} can work on empty files). > QuidemTests are not effectively executed on Windows > --- > > Key: CALCITE-5980 > URL: https://issues.apache.org/jira/browse/CALCITE-5980 > Project: Calcite > Issue Type: Bug > Components: core >Affects Versions: 1.35.0 >Reporter: Ruben Q L >Assignee: Ruben Q L >Priority: Major > Fix For: 1.36.0 > > > Discovered by accident on my Windows+IntelliJ environment while I was trying > to add new tests on a *.iq file. My new tests did not seem to be executed. I > even tried adding syntax errors on purpose into different iq files to force > them to fail, but the tests were still successful. The reason seems to be > that, at least on my environment (Windows), the test files do not execute any > of their statements. This seems caused by the changes introduced in > CALCITE-5786. > While debugging, I found this line in QuidemTest.java (that aims to create > the inFile and outFile): > {code:java} > protected void checkRun(String path) throws Exception { > ... > // e.g. path = "sql/agg.iq" > // inUrl = "file:/home/fred/calcite/core/build/resources/test/sql/agg.iq" > // inFile = "/home/fred/calcite/core/build/resources/test/sql/agg.iq" > // outDir = "/home/fred/calcite/core/build/quidem/test/sql" > // outFile = "/home/fred/calcite/core/build/quidem/test/sql/agg.iq" > inFile = Sources.of(requireNonNull(inUrl, "inUrl")).file(); > outFile = replaceDir(inFile, "resources", "quidem"); > ... > } > {code} > But in my case it results on {*}both files being the same{*}, thus when the > outFile is created, it actually erases all the tests that were contained > inside the inFile, so the test runs nothing. > The reason for that is that the auxiliary method {{{}replaceDir{}}}: > {code:java} > private static File replaceDir(File file, String target, String > replacement) { > return new File( > file.getAbsolutePath().replace(n2u('/' + target + '/'), > n2u('/' + replacement + '/'))); > } > {code} > is trying to replace "/resources/" with "/quidem/" from the inFile absolute > path, but in my case this path does not contain the pattern to be replaced, > since it contains backslashes: > "C:\...\calcite\core\build\resources\test\sql\agg.iq"; so the replacement > operation does nothing. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (CALCITE-5980) QuidemTests are not effectively executed on Windows
[ https://issues.apache.org/jira/browse/CALCITE-5980?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17762596#comment-17762596 ] Ruben Q L commented on CALCITE-5980: What about checking that each inFile must not be empty? That check fails on current master, but passes on my branch. > QuidemTests are not effectively executed on Windows > --- > > Key: CALCITE-5980 > URL: https://issues.apache.org/jira/browse/CALCITE-5980 > Project: Calcite > Issue Type: Bug > Components: core >Affects Versions: 1.35.0 >Reporter: Ruben Q L >Assignee: Ruben Q L >Priority: Major > Fix For: 1.36.0 > > > Discovered by accident on my Windows+IntelliJ environment while I was trying > to add new tests on a *.iq file. My new tests did not seem to be executed. I > even tried adding syntax errors on purpose into different iq files to force > them to fail, but the tests were still successful. The reason seems to be > that, at least on my environment (Windows), the test files do not execute any > of their statements. This seems caused by the changes introduced in > CALCITE-5786. > While debugging, I found this line in QuidemTest.java (that aims to create > the inFile and outFile): > {code:java} > protected void checkRun(String path) throws Exception { > ... > // e.g. path = "sql/agg.iq" > // inUrl = "file:/home/fred/calcite/core/build/resources/test/sql/agg.iq" > // inFile = "/home/fred/calcite/core/build/resources/test/sql/agg.iq" > // outDir = "/home/fred/calcite/core/build/quidem/test/sql" > // outFile = "/home/fred/calcite/core/build/quidem/test/sql/agg.iq" > inFile = Sources.of(requireNonNull(inUrl, "inUrl")).file(); > outFile = replaceDir(inFile, "resources", "quidem"); > ... > } > {code} > But in my case it results on {*}both files being the same{*}, thus when the > outFile is created, it actually erases all the tests that were contained > inside the inFile, so the test runs nothing. > The reason for that is that the auxiliary method {{{}replaceDir{}}}: > {code:java} > private static File replaceDir(File file, String target, String > replacement) { > return new File( > file.getAbsolutePath().replace(n2u('/' + target + '/'), > n2u('/' + replacement + '/'))); > } > {code} > is trying to replace "/resources/" with "/quidem/" from the inFile absolute > path, but in my case this path does not contain the pattern to be replaced, > since it contains backslashes: > "C:\...\calcite\core\build\resources\test\sql\agg.iq"; so the replacement > operation does nothing. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (CALCITE-5980) QuidemTests are not effectively executed on Windows
[ https://issues.apache.org/jira/browse/CALCITE-5980?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17762480#comment-17762480 ] Julian Hyde commented on CALCITE-5980: -- Oh boy, good catch. I should have known that something was up when it 'just worked' on Windows. > QuidemTests are not effectively executed on Windows > --- > > Key: CALCITE-5980 > URL: https://issues.apache.org/jira/browse/CALCITE-5980 > Project: Calcite > Issue Type: Bug > Components: core >Affects Versions: 1.35.0 >Reporter: Ruben Q L >Assignee: Ruben Q L >Priority: Major > Fix For: 1.36.0 > > > Discovered by accident on my Windows+IntelliJ environment while I was trying > to add new tests on a *.iq file. My new tests did not seem to be executed. I > even tried adding syntax errors on purpose into different iq files to force > them to fail, but the tests were still successful. The reason seems to be > that, at least on my environment (Windows), the test files do not execute any > of their statements. This seems caused by the changes introduced in > CALCITE-5786. > While debugging, I found this line in QuidemTest.java (that aims to create > the inFile and outFile): > {code:java} > protected void checkRun(String path) throws Exception { > ... > // e.g. path = "sql/agg.iq" > // inUrl = "file:/home/fred/calcite/core/build/resources/test/sql/agg.iq" > // inFile = "/home/fred/calcite/core/build/resources/test/sql/agg.iq" > // outDir = "/home/fred/calcite/core/build/quidem/test/sql" > // outFile = "/home/fred/calcite/core/build/quidem/test/sql/agg.iq" > inFile = Sources.of(requireNonNull(inUrl, "inUrl")).file(); > outFile = replaceDir(inFile, "resources", "quidem"); > ... > } > {code} > But in my case it results on {*}both files being the same{*}, thus when the > outFile is created, it actually erases all the tests that were contained > inside the inFile, so the test runs nothing. > The reason for that is that the auxiliary method {{{}replaceDir{}}}: > {code:java} > private static File replaceDir(File file, String target, String > replacement) { > return new File( > file.getAbsolutePath().replace(n2u('/' + target + '/'), > n2u('/' + replacement + '/'))); > } > {code} > is trying to replace "/resources/" with "/quidem/" from the inFile absolute > path, but in my case this path does not contain the pattern to be replaced, > since it contains backslashes: > "C:\...\calcite\core\build\resources\test\sql\agg.iq"; so the replacement > operation does nothing. -- This message was sent by Atlassian Jira (v8.20.10#820010)