[Webkit-unassigned] [Bug 125607] Test new JSContext name APIs

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=125607


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #219021|review?                     |review+
               Flag|                            |




--- Comment #2 from Darin Adler <darin at apple.com>  2013-12-12 00:28:06 PST ---
(From update of attachment 219021)
View in context: https://bugs.webkit.org/attachment.cgi?id=219021&action=review

> Source/JavaScriptCore/API/tests/testapi.c:1089
> +    JSGlobalContextRef context = JSGlobalContextCreate(0);

nullptr

> Source/JavaScriptCore/API/tests/testapi.c:1092
> +    result &= assertTrue(!str, "Default context name is NULL");

Not sure we should capitalize NULL just because there’s a C macro that we never use by that name. I think the word null is fine without shouting.

> Source/JavaScriptCore/API/tests/testapi.mm:1203
> +        JSContext *context = [[JSContext alloc] init];
> +        checkResult(@"default context.name is nil", context.name == nil);
> +        context.name = @"Foo";
> +        checkResult(@"set context.name is expected", [context.name isEqualToString:@"Foo"]);

Why less testing here? Why not replicate the “two names” thing you did in the C test?

> Source/JavaScriptCore/ChangeLog:9
> +        * API/JSContext.h:
> +        * API/JSContextRef.h:

Really ought to say why you touched these files. Obviously not to “test new JSContext name APIs”.

> Tools/ChangeLog:8
> +        Test new JSContext name APIs
> +        https://bugs.webkit.org/show_bug.cgi?id=125607
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        * Scripts/run-javascriptcore-tests:

Change has nothing to do with the bug title.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.