let me see if I understand this problem correctly: We currently save the name and the case of a PageName in the name of file (2.8) or the name of a directory (3.0 / priha), right ? If you run on a platform that is not (completely) case-sensitive with regard to file/dir names, this get's us into trouble, right ? If that's the case and we want the preserve platform neutrality, we should use something else to hold the name and case of pagenames, and wiki:title seems a proper solution for that.
/Harry 2009/10/31 Janne Jalkanen <[email protected]> > Yup, getting back on this... > > It seems to me that the following patch fixes the issue, BUT it does expose > quite a lot of other problems, mostly related to the fact that JSPWiki (and > our tests) assume that page names stay in the same case as it was created. > However, if we lowercase all page names as they are stored, the case > information is lost. If we do not, then there's no way that we can really > get the JCR name from an arbitrarily cased page name. > > One possibility is to add a new attribute, "wiki:title", which would > contain the page name as originally created, and would be the "display" > name. > > /Janne > > ### Eclipse Workspace Patch 1.0 > #P JSPWiki > Index: src/java/org/apache/wiki/content/WikiPath.java > =================================================================== > --- src/java/org/apache/wiki/content/WikiPath.java (revision 831483) > +++ src/java/org/apache/wiki/content/WikiPath.java (working copy) > @@ -193,22 +193,24 @@ > * String representation. This is to fulfil the general contract of > * equals(). > * > - * @return int > + * @return {...@inheritdoc} > */ > + // FIXME: Slow, since it creates a new String every time. > public int hashCode() > { > - return m_stringRepresentation.hashCode(); > + return m_stringRepresentation.toLowerCase().hashCode(); > } > > /** > * A WikiPath is compared using it's toString() method. > * > * @param o The Object to compare against. > - * @return int > + * @return {...@inheritdoc} > */ > + // FIXME: Slow, since it creates a new String every time. > public int compareTo( WikiPath o ) > { > - return toString().compareTo( o.toString() ); > + return toString().toLowerCase().compareTo( > o.toString().toLowerCase() ); > } > > /** > @@ -226,11 +228,11 @@ > { > WikiPath n = (WikiPath) o; > > - return m_space.equals( n.m_space ) && m_path.equals( n.m_path > ); > + return m_space.equalsIgnoreCase( n.m_space ) && > m_path.equalsIgnoreCase( n.m_path ); > } > else if( o instanceof String ) > { > - return toString().equals( o ); > + return toString().equalsIgnoreCase( (String)o ); > } > return false; > } > Index: src/java/org/apache/wiki/content/ContentManager.java > =================================================================== > --- src/java/org/apache/wiki/content/ContentManager.java (revision > 831483) > +++ src/java/org/apache/wiki/content/ContentManager.java (working > copy) > @@ -113,7 +113,7 @@ > */ > public static final String DEFAULT_SPACE = "Main"; > > - private static final String JCR_DEFAULT_SPACE = > "pages/"+DEFAULT_SPACE; > + private static final String JCR_DEFAULT_SPACE = > "pages/"+DEFAULT_SPACE.toLowerCase(); > > private static final String JCR_PAGES_NODE = "pages"; > > @@ -1433,8 +1433,8 @@ > String spaceName; > String spacePath; > > - spaceName = wikiName.getSpace(); > - spacePath = wikiName.getPath(); > + spaceName = wikiName.getSpace().toLowerCase(); > + spacePath = wikiName.getPath().toLowerCase(); > > return "/"+JCR_PAGES_NODE+"/"+spaceName+"/"+spacePath; > > } > > > On Oct 28, 2009, at 22:25 , Janne Jalkanen wrote: > > >> Hm. This seems to be a bug in JSPWiki, actually. JCR Names *are* case >> sensitive, so we should actually be normalizing the name to "this is a >> test". The fact that it's created with an upper-case name suggests that >> JSPWiki is not normalizing the name properly. >> >> /Janne >> >> On Oct 28, 2009, at 20:17 , Harry Metske wrote: >> >> frustrated by the one remaining failing unit test I took a closer look at >>> why WikiEngineTest.testSpacedNames1() fails. >>> I can only conclude that 2 of the 3 tests are wrong. >>> First a page is created with name "This is a test", this eventually >>> resulting in a directory being created somewhere deep down the priha repo >>> : >>> >>> mets...@gneisenau >>> ~/workspace/JSPWiki/build/tests/priha/workspaces/jspwiki/pages/Main >>> $ ls -l >>> total 12 >>> -rw-r--r-- 1 metskem metskem 43 2009-10-28 17:55 jcr:primaryType.data >>> -rw-r--r-- 1 metskem metskem 82 2009-10-28 17:55 jcr:primaryType.info >>> drwxr-xr-x 2 metskem metskem 4096 2009-10-28 19:07 This is a test >>> >>> Then we do 3 tests: >>> >>> assertEquals( "normal", "puppaa", m_engine.getText("This is a >>> test").trim() ); >>> assertEquals( "lowercase", "puppaa", m_engine.getText("this is a >>> test").trim() ); >>> assertEquals( "randomcase", "puppaa", m_engine.getText("ThiS Is a >>> teSt").trim() ); >>> >>> Only the first one succeeds, the second and third one fail ( of course I >>> would say), at least on Linux with priha, file/dir names are case >>> sensitive. >>> >>> What am I missing, and if nothing, can I remove the last two tests ? >>> >>> thanks, >>> Harry >>> >>> 2009/10/27 Harry Metske <[email protected]> >>> >>> I just did a fresh svn checkout, and ran "ant clean tests" from the >>>> cmdline. >>>> This gives me 3 failures and 13 errors: >>>> http://www.computerhok.nl/tmp/junit-noframes.html >>>> >>>> The WikiEngineTest.testSpacedNames1() always fails (Linux versus >>>> Mac/Windows) >>>> >>>> Here's an overview of more tests : >>>> http://www.computerhok.nl/tmp/jspwiki-testresult.html >>>> >>>> regards, >>>> Harry >>>> >>>> 2009/10/27 Andrew Jaquith <[email protected]> >>>> >>>> Sounds like we have a few issues here: >>>> >>>>> >>>>> 1) Guitests. I'll see what I can find. Probably something minor. I >>>>> know the "tests" target runs all test classes ending in "*Test" and >>>>> ignores "AllTests", while Eclipse (and probably guitests) just runs >>>>> the AllTests classes. It's likely that one or more of the AllTests >>>>> classes is failing to include, oh, about 34 tests. :) >>>>> >>>>> 2) Graceful LDAP fail (inside the tests themselves). Any ideas on how >>>>> to implement? The easy way would be to look for a localhost listener >>>>> on 4890 (where the OpenLDAP test fixture listens) and then not run the >>>>> tests if it isn't found. Should they FAIL or PASS in that case? It >>>>> sounds like passing is the right thing to do. >>>>> >>>>> 3) Differences in your test pass rate versus mine. Not sure why your >>>>> "ant tests" run would produce different results than mine. I did try >>>>> running mine with a completely new, checked-out branch. Because I >>>>> can't know what changes you might have in your local branch, could you >>>>> check out a clean copy and diff the tree versus yours? SOMETHING is >>>>> different. Also, I'd like to know what Harry and others are seeing. >>>>> Gents, any clues? >>>>> >>>>> I agree that all three methods should return the same number of test >>>>> cases, and pass/fail the same ways. I also agree that tests should be >>>>> self-contained. That was part of the rationale for the Ant script >>>>> tweaks I checked in recently. >>>>> >>>>> Eclipse, by the way, hasn't been reliable for me, for testing, for a >>>>> while. I tend to exhaust memory somewhere around JSPWikiMarkupParser. >>>>> But I haven't tried it in the last few months (i.e. before my massive >>>>> bug-hunting campaign). >>>>> >>>>> Andrew >>>>> >>>>> >>>>> >>>>> On Tue, Oct 27, 2009 at 3:25 AM, Janne Jalkanen >>>>> <[email protected]> wrote: >>>>> >>>>>> Interestingly, I applied your most recent checkins applied (and I have >>>>>>> small one patch to JSPWikiMarkupParserTest that I haven't checked >>>>>>> in). >>>>>>> I am running 100% clean, with no errors. Total number of tests: 1024 >>>>>>> -- a nice round number. :) WikiEngineTest.testOldVersionVars has >>>>>>> been running fine for me for a while. >>>>>>> >>>>>> >>>>>> There's no way it should've run, unless you have some old code/config >>>>>> >>>>> files >>>>> >>>>>> lying around. Can you check out a previous version to a clean >>>>>> directory >>>>>> >>>>> and >>>>> >>>>>> see if it runs? >>>>>> >>>>>> As a control case, I also checked out a new built from trunk, and >>>>>>> simply typed 'ant tests'. I used a vanilla build with absolutely no >>>>>>> customizations, even to build.properties. It ran completely clean >>>>>>> also >>>>>>> except for 1 JSPWikiMarkupParserTest test (because I haven't checked >>>>>>> in that fix), 1024 tests total. >>>>>>> >>>>>> >>>>>> Running the AllTests from Eclipse or with "ant guitests" results in >>>>>> 990 >>>>>> >>>>> test >>>>> >>>>>> cases. "ant tests" is the only one giving 1024 tests, and I get 12 >>>>>> >>>>> failures >>>>> >>>>>> and 14 errors for it. LdapAuthorizerTest, LdapUserDatabaseTest and >>>>>> XMLUserDatabaseTest all fail with all tests. >>>>>> >>>>>> What I find odd is that guitests and tests targets should give the >>>>>> same >>>>>> results, since they both are run from build.xml. >>>>>> >>>>>> The only other item causing the discrepancy would be if you don't >>>>>>> have >>>>>>> a local LDAP server running for the LDAP tests. Those should cause, >>>>>>> at >>>>>>> most, 14 failures or errors. I'll add in some code to build.xml to >>>>>>> set >>>>>>> up the LDAP fixtures and/or disable the tests if the OpenLDAP >>>>>>> executable isn't available. >>>>>>> >>>>>> >>>>>> I think it's probably a better idea to do the test directly in the >>>>>> tests >>>>>> itself. The JCR TCK throws a NonExecutableException when the test >>>>>> case >>>>>> cannot be executed (and this shows up as a passed test). >>>>>> >>>>>> I think it's important that all three methods give the same number of >>>>>> >>>>> test >>>>> >>>>>> cases; if the number is not reliable, it's too easy to forget to run >>>>>> >>>>> certain >>>>> >>>>>> tests. >>>>>> >>>>>> Also, I sometimes run all tests for a given package from within >>>>>> Eclipse. >>>>>> >>>>> I'd >>>>> >>>>>> like the test cases to be self-contained enough so that I don't have >>>>>> to >>>>>> remember which tests are supposed to run under which conditions. >>>>>> >>>>>> /Janne >>>>>> >>>>>> >>>>> >>>> >>>> >
