On 12/04/2019 09:56, Mark Thomas wrote: > On 12/04/2019 01:32, Igal Sapir wrote: >> Mark, >> >> I just realized that you refactored my fix for the false positive unit test >> [1] a while ago. Unfortunately your refactoring still shows false >> positives if the path of the command prompt has a lower-cased driver >> letter, e.g. >> >> expected:<jar:war:file:/[E]:/Workspace/Test/apa...> but >> was:<jar:war:file:/[e]:/Workspace/Test/apa...> >> >> >> I pushed a different fix earlier, prior to noticing your refactoring (TBH I >> thought that I never pushed the fix in the first place), which does an >> equalsIgnoreCase() [2] >> >> The only unit tests where I experience this issue >> are TestAbstractArchiveResource.java and TestFileResource.java and the >> problem seems to be with the Driver Letter which appears in brackets, e.g. >> [E] vs. [e]. >> >> If you think that a full case-insensitive comparison is not right then I >> can modify only the drive letter if the pattern "jar:war:file:/[X]" is >> found. >> >> Any thoughts? > > The test failure indicates that something isn't using canonical paths > when it should. Lets see if we can figure out where and then decide how > best to fix it. > > For reference, as well as refactoring the tests I made this change: > https://github.com/apache/tomcat/commit/d63695a656f04e39bd1ad4dee0f2339b0e3b898f#diff-ec06eb37e0fee8269d835107dedf7a90 > > which triggered a regression hence this additional change: > https://github.com/apache/tomcat/commit/ad60947e42e666dc9c9d77315787ea9bb567e3fd#diff-ec06eb37e0fee8269d835107dedf7a90 > > I wonder if this failure is related. > > I'm going to look at this now. If it doesn't take too long, I;d like to > get this fixed before 9.0.19.
The bug is in ContextConfig but it is proving a little tricky to address as there are lots of edge cases where we need to be careful. I'll leave the current work-around for the test in place and look at this again after the 9.0.19 tag. Mark > > Mark > > >> >> Igal >> >> [1] >> https://github.com/apache/tomcat/commit/db71c925106915581f1b60b0fda9c352fcdd9138#diff-717001e0451788fe9f26d1176a4fff54 >> >> [2] >> https://github.com/apache/tomcat/commit/985d0086329c012a994c842f0a88a9b33989827c#diff-717001e0451788fe9f26d1176a4fff54R43 >> >> >> >> On Thu, Apr 11, 2019 at 11:28 AM <isa...@apache.org> wrote: >> >>> This is an automated email from the ASF dual-hosted git repository. >>> >>> isapir pushed a commit to branch master >>> in repository https://gitbox.apache.org/repos/asf/tomcat.git >>> >>> >>> The following commit(s) were added to refs/heads/master by this push: >>> new 985d008 Fixed false positives in unit tests on Windows >>> 985d008 is described below >>> >>> commit 985d0086329c012a994c842f0a88a9b33989827c >>> Author: Igal Sapir <isa...@apache.org> >>> AuthorDate: Thu Apr 11 08:45:22 2019 -0700 >>> >>> Fixed false positives in unit tests on Windows >>> >>> When the drive letter is lower cased in a Windows command prompt the >>> test cases were failing with >>> expected:<jar:war:file:/[E]:/Workspace/Test/apa...> but >>> was:<jar:war:file:/[e]:/Workspace/Test/apa...> >>> --- >>> .../org/apache/catalina/webresources/TestAbstractArchiveResource.java | 4 >>> ++-- >>> test/org/apache/catalina/webresources/TestFileResource.java | 2 >>> +- >>> 2 files changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git >>> a/test/org/apache/catalina/webresources/TestAbstractArchiveResource.java >>> b/test/org/apache/catalina/webresources/TestAbstractArchiveResource.java >>> index 9c59fd8..e573d91 100644 >>> --- >>> a/test/org/apache/catalina/webresources/TestAbstractArchiveResource.java >>> +++ >>> b/test/org/apache/catalina/webresources/TestAbstractArchiveResource.java >>> @@ -48,7 +48,7 @@ public class TestAbstractArchiveResource extends >>> TomcatBaseTest { >>> >>> expectedURL.append(docBase.getCanonicalFile().toURI().toURL().toString()); >>> >>> >>> expectedURL.append("*/WEB-INF/lib/test.jar!/META-INF/resources/index.html"); >>> >>> - Assert.assertEquals(expectedURL.toString(), >>> webResource.getURL().toString()); >>> + >>> Assert.assertTrue(expectedURL.toString().equalsIgnoreCase(webResource.getURL().toString())); >>> } >>> >>> >>> @@ -71,7 +71,7 @@ public class TestAbstractArchiveResource extends >>> TomcatBaseTest { >>> >>> expectedURL.append(docBase.getCanonicalFile().toURI().toURL().toString()); >>> >>> expectedURL.append("WEB-INF/lib/test-lib.jar!/META-INF/tags/echo.tag"); >>> >>> - Assert.assertEquals(expectedURL.toString(), >>> webResource.getURL().toString()); >>> + >>> Assert.assertTrue(expectedURL.toString().equalsIgnoreCase(webResource.getURL().toString())); >>> } >>> >>> } >>> diff --git a/test/org/apache/catalina/webresources/TestFileResource.java >>> b/test/org/apache/catalina/webresources/TestFileResource.java >>> index 315212a..2bd7ef3 100644 >>> --- a/test/org/apache/catalina/webresources/TestFileResource.java >>> +++ b/test/org/apache/catalina/webresources/TestFileResource.java >>> @@ -40,6 +40,6 @@ public class TestFileResource extends TomcatBaseTest { >>> >>> // Build the expected location the same way the webapp base dir >>> is built >>> File f = new File("test/webapp/WEB-INF/classes"); >>> - >>> Assert.assertEquals(f.getCanonicalFile().toURI().toURL().toString(), >>> out.toString().trim()); >>> + >>> Assert.assertTrue(f.getCanonicalFile().toURI().toURL().toString().equalsIgnoreCase(out.toString().trim())); >>> } >>> } >>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org >>> For additional commands, e-mail: dev-h...@tomcat.apache.org >>> >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
