incubator-jspwiki-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Harry Metske <harry.met...@gmail.com>
Subject Re: 2 last failing unit tests
Date Sun, 01 Nov 2009 11:52:53 GMT
+1 for me for becoming completely case-sensitive, I understand that this
will break compatibility with 2.8 a bit more, but it would makes things
simpler I think.

BTW: Is the same discussion true for camelCaseLinks and matchEnglishPlurals,
and if so, should we drop that too ?

/Harry

2009/10/31 Janne Jalkanen <Janne.Jalkanen@ecyrd.com>

>
> Almost. JCR is by definition case-sensitive, and the underlying
> implementation enforces that (the pages could very well be in a JDBC
> database). So the exact details how Priha stores the data is unimportant.
>
> The problem is that JSPWiki as currently written is case-sensitive, which
> means that using links like [this page] and [This page] and [This Page]
> actually refer to different pages. We just do some magic where we normalize
> the page names to some casing rules, but that is breaking down right now,
> and is not really case-insensitive.  It's just a guess and causes all sorts
> of interesting problems like with ReferenceManager (e.g. when the page does
> not exist, what exactly should the right spelling be?)
>
> So I think the correct solution for 3.0 should be just to define that page
> names are case insensitive.
>
> (To convolute the thing slightly, in English a normalization where all
> words are capitalized, e.g. "This Is A Page" is okay, since that's the way
> you write headlines.  However, in Finnish this kind of normalization is
> simply wrong and looks really bad.  Case insensitivity solves this problem
> neatly.)
>
> /Janne
>
>
> On Oct 31, 2009, at 20:29 , Harry Metske wrote:
>
>  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 <Janne.Jalkanen@ecyrd.com>
>>
>>  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
>>>>> :
>>>>>
>>>>> metskem@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 <harry.metske@gmail.com>
>>>>>
>>>>> 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 <andrew.r.jaquith@gmail.com>
>>>>>>
>>>>>> 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
>>>>>>> <Janne.Jalkanen@ecyrd.com> 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
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message