harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nathan Beyer <ndbe...@apache.org>
Subject Re: [M10] Testing
Date Fri, 29 May 2009 01:13:29 GMT
On Thu, May 28, 2009 at 8:28 AM, Oliver Deakin
<oliver.deakin@googlemail.com> wrote:
> Nathan Beyer wrote:
>>
>> I'd like to apply the final patch attached to
>> https://issues.apache.org/jira/browse/HARMONY-6074. This seems to
>> completely resolve the Maven test run issue.
>>
>
> Ok - since it seems the original patch applied at the end of the last week
> does not completely fix the issue, and our aim was to get the Maven tests
> running on M10, I'm +1 to this commit as well. I've applied the patch and
> run the luni tests and they all pass, so please go ahead and commit the
> patch as soon as possible so we can restart the testing process.

Applied.

>
>> As for testing, I'm not seeing the hymmap issue on Windows Vista x86.
>> However, the tests pass with and without this patch.
>>
>
> I've had the tests pass in the past as well, but there is definitely an
> error in the test case - what I see is that after the strncpy() on line 132
> of the hymmap test case we get an extra character at the end of the path.
> From looking at the msdn pages it looks like strncpy() does not add a null
> character to the end of the copied string if you are not copying the entire
> source string, which is the case here. Now I have had a closer look I think
> the +2 is actually correct, because we add _another_ slash character into
> the path further down, and in fact we simply need to make sure the string is
> null terminated after the strncpy() call so the strcat() call starts in the
> right place [1]. This patch makes the test behave as expected for me. As I
> said in the other branch of this thread, I don't feel this needs to be fixed
> for M10 so Ill hold off committing it for now.
>
> Regards,
> Oliver

This sounds good to me.

>
> [1]
> Index: hymmap.c
> ===================================================================
> --- hymmap.c    (revision 779066)
> +++ hymmap.c    (working copy)
> @@ -130,6 +130,7 @@
>    testFile = hyportLibrary->mem_allocate_memory(hyportLibrary, pathLen +
> strlen("shared") + 2 + strlen("testFile"));
>
>    strncpy(emptyFile, execPath, pathLen);
> +    emptyFile[pathLen] = '\0';
>    strcat(emptyFile, "shared");
>    strcat(emptyFile, DIR_SEPARATOR_STR);
>    strcpy(testFile, emptyFile);
>
>> -Nathan
>>
>> On Wed, May 27, 2009 at 10:24 AM, Oliver Deakin
>> <oliver.deakin@googlemail.com> wrote:
>>
>>>
>>> Hi all,
>>>
>>> I've just run the classlib tests on Windows x86 on the latest code in the
>>> federated build and I don't see anything catastrophic there. There's 1
>>> failure in the portlib tests, in hymmap, which I think is a test case
>>> off-by-one error. I've attached a patch [1] which fixes the test. Apart
>>> from
>>> that we look clean.
>>>
>>> Does anyone have any other test reports? Any other platforms? I'm
>>> assuming
>>> we're taking r778555 as our testing base for M10.
>>>
>>> Regards,
>>> Oliver
>>>
>>> [1]
>>> Adding 2 to the path size for emptyFile and testFile is incorrect, as
>>> pathLen already counts the rightmost DIR_SEPARATOR.
>>>
>>> Index: modules/portlib/src/test/native/hymmap/shared/hymmap.c
>>> ===================================================================
>>> --- modules/portlib/src/test/native/hymmap/shared/hymmap.c    (revision
>>> 779066)
>>> +++ modules/portlib/src/test/native/hymmap/shared/hymmap.c    (working
>>> copy)
>>> @@ -126,8 +126,8 @@
>>>   execName = strrchr(execPath, DIR_SEPARATOR) + 1; /* Find the rightmost
>>> slash character */
>>>   pathLen = strlen(execPath) - strlen(execName);
>>>
>>> -    emptyFile = hyportLibrary->mem_allocate_memory(hyportLibrary,
>>> pathLen +
>>> strlen("shared") + 2 + strlen("emptyFile")); /* +2 for the extra slash
>>> and
>>> null terminator */
>>> -    testFile = hyportLibrary->mem_allocate_memory(hyportLibrary, pathLen
>>> +
>>> strlen("shared") + 2 + strlen("testFile"));
>>> +    emptyFile = hyportLibrary->mem_allocate_memory(hyportLibrary,
>>> pathLen +
>>> strlen("shared") + 1 + strlen("emptyFile")); /* +1 for the null
>>> terminator
>>> */
>>> +    testFile = hyportLibrary->mem_allocate_memory(hyportLibrary, pathLen
>>> +
>>> strlen("shared") + 1 + strlen("testFile"));
>>>
>>>   strncpy(emptyFile, execPath, pathLen);
>>>   strcat(emptyFile, "shared");
>>>
>>> --
>>> Oliver Deakin
>>> Unless stated otherwise above:
>>> IBM United Kingdom Limited - Registered in England and Wales with number
>>> 741598. Registered office: PO Box 41, North Harbour, Portsmouth,
>>> Hampshire
>>> PO6 3AU
>>>
>>>
>>>
>>
>>
>
> --
> Oliver Deakin
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
> PO6 3AU
>
>

Mime
View raw message