Bug 125607 – Test new JSContext name APIs

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Joseph Pecoraro]

I should have written a test when I added these APIs. Better late then never.

[Joseph Pecoraro]

Created attachment 219021 [details]
[PATCH] Add Test

[Darin Adler]

Comment on attachment 219021 [details]
[PATCH] Add Test

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.

[Joseph Pecoraro]

Landed r160494: <http://trac.webkit.org/changeset/160494>