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 Sat, 31 Oct 2009 18:29:51 GMT
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