Bug 148723 - [WK2] Allow tagging tests with metadata which needs to be known at web process creation time
Summary: [WK2] Allow tagging tests with metadata which needs to be known at web proces...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-02 14:13 PDT by Myles C. Maxfield
Modified: 2015-09-03 13:22 PDT (History)
1 user (show)

See Also:


Attachments
Patch (3.92 KB, patch)
2015-09-02 14:13 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (4.00 KB, patch)
2015-09-02 14:33 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (4.19 KB, patch)
2015-09-02 14:54 PDT, Myles C. Maxfield
andersca: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2015-09-02 14:13:26 PDT
[WK2] Allow tagging tests with metadata which needs to be known at web process creation time
Comment 1 Myles C. Maxfield 2015-09-02 14:13:48 PDT
Created attachment 260436 [details]
Patch
Comment 2 Myles C. Maxfield 2015-09-02 14:33:37 PDT
Created attachment 260441 [details]
Patch
Comment 3 WebKit Commit Bot 2015-09-02 14:36:36 PDT
Attachment 260441 [details] did not pass style-queue:


ERROR: Tools/WebKitTestRunner/TestController.cpp:57:  Streams are highly discouraged.  [readability/streams] [3]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Anders Carlsson 2015-09-02 14:39:36 PDT
Comment on attachment 260441 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=260441&action=review

> Tools/WebKitTestRunner/TestController.cpp:801
> +        auto length = WKStringGetLength(path.get());
> +        WKChar buffer[length];

This should use a vector instead of a variable-length array.

Also, I think you can just use 

WK_EXPORT size_t WKStringGetMaximumUTF8CStringSize(WKStringRef string);
WK_EXPORT size_t WKStringGetUTF8CString(WKStringRef string, char* buffer, size_t bufferSize);

and return an std::string here.

> Tools/WebKitTestRunner/TestController.cpp:814
> +    // Do everything using file IO so we don't mess up caching on HTTP tests.

I don't think we need this comment.

> Tools/WebKitTestRunner/TestController.cpp:827
> +    if (endLocation == std::string::npos)
> +        return;

I think we should LOG_ERROR when we fail to parse this.
Comment 5 Myles C. Maxfield 2015-09-02 14:54:40 PDT
Created attachment 260443 [details]
Patch
Comment 6 WebKit Commit Bot 2015-09-02 14:57:38 PDT
Attachment 260443 [details] did not pass style-queue:


ERROR: Tools/WebKitTestRunner/TestController.cpp:57:  Streams are highly discouraged.  [readability/streams] [3]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 WebKit Commit Bot 2015-09-02 18:10:41 PDT
Comment on attachment 260443 [details]
Patch

Rejecting attachment 260443 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 260443, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
', 'dcommit', '--rmdir']" exit_code: 139 cwd: /Volumes/Data/EWS/WebKit

Committing to http://svn.webkit.org/repository/webkit/trunk ...
Authentication realm: <http://svn.webkit.org:80> Mac OS Forge
Password for 'commit-queue@webkit.org': 
Authentication realm: <http://svn.webkit.org:80> Mac OS Forge
Username: error: git-svn died of signal 11

Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 139 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
Current branch master is up to date.

Full output: http://webkit-queues.webkit.org/results/134766
Comment 8 Myles C. Maxfield 2015-09-03 13:21:36 PDT
http://trac.webkit.org/changeset/189283