Bug 142083 - Layout tests imported/w3c/web-platform-tests certificates should not be tracked on WebKit VCS
Summary: Layout tests imported/w3c/web-platform-tests certificates should not be track...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
: 142136 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-02-27 04:00 PST by Carlos Alberto Lopez Perez
Modified: 2015-03-06 10:37 PST (History)
9 users (show)

See Also:


Attachments
Patch (27.20 KB, patch)
2015-02-27 04:03 PST, Carlos Alberto Lopez Perez
simon.fraser: review-
simon.fraser: commit-queue-
Details | Formatted Diff | Diff
Moving _certs location to layout-test-results folder (30.06 KB, patch)
2015-03-03 04:43 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (30.05 KB, patch)
2015-03-06 00:20 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (29.95 KB, patch)
2015-03-06 01:14 PST, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2015-02-27 04:00:12 PST
After running:

$ Tools/Scripts/run-webkit-tests --release --webkit-test-runner --gtk imported/w3c/web-platform-tests
Using port 'gtk-wk2'
Test configuration: <, x86, release>
Placing test results in /home/clopez/webkit/webkit/WebKitBuild/Release/layout-test-results
Baseline search path: gtk -> wk2 -> generic
Using Release build
Pixel tests disabled
Regular timeout: 6000, slow test timeout: 30000
Command line: /home/clopez/webkit/webkit/Tools/jhbuild/jhbuild-wrapper --gtk run /home/clopez/webkit/webkit/WebKitBuild/Release/bin/WebKitTestRunner -

Found 20 tests; running 2, skipping 18.
Running 1 WebKitTestRunner.          

All 2 tests ran as expected.                     



The test modified some certificate files that are tracked on the VCS system (git) and causes unwanted noise.

$ git st
On branch master
Your branch is up-to-date with 'origin/master'.
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   LayoutTests/imported/w3c/web-platform-tests/_certs/01.pem
	modified:   LayoutTests/imported/w3c/web-platform-tests/_certs/02.pem
	modified:   LayoutTests/imported/w3c/web-platform-tests/_certs/cacert.pem
	modified:   LayoutTests/imported/w3c/web-platform-tests/_certs/cakey.pem
	modified:   LayoutTests/imported/w3c/web-platform-tests/_certs/index.txt.old
	modified:   LayoutTests/imported/w3c/web-platform-tests/_certs/localhost.key
	modified:   LayoutTests/imported/w3c/web-platform-tests/_certs/localhost.pem
	modified:   LayoutTests/imported/w3c/web-platform-tests/_certs/serial
Comment 1 Carlos Alberto Lopez Perez 2015-02-27 04:03:33 PST
Created attachment 247508 [details]
Patch
Comment 2 Carlos Alberto Lopez Perez 2015-02-27 04:06:46 PST
The patch removes all the files under _certs and adds the whole _certs directory to .gitignore


When runing the test after this patch, the test creates again the certificates and passes as expected, but since the files are not longer tracked and are also ignored on .gitignore they not longer appear as uncommitted/modified files to git.
Comment 3 youenn fablet 2015-02-27 04:53:50 PST
(In reply to comment #2)
> The patch removes all the files under _certs and adds the whole _certs
> directory to .gitignore
> 
> 
> When runing the test after this patch, the test creates again the
> certificates and passes as expected, but since the files are not longer
> tracked and are also ignored on .gitignore they not longer appear as
> uncommitted/modified files to git.

The certificates were originally checked in the repo to ensure repeatability of the tests. I am not sure why they were regenerated.
At this time, removing them and ignoring them may be just fine.

It might be slightly better to generate them in WebKitBuild/.../layout-test-results. This may be done by updating the field ssl.open_ssl.base_path (see LayoutTests/imported/w3c/resources/config.json) when starting the WPT server.
This can be added in the future if that proves to be useful.
Comment 4 youenn fablet 2015-02-28 13:47:06 PST
> It might be slightly better to generate them in
> WebKitBuild/.../layout-test-results. This may be done by updating the field
> ssl.open_ssl.base_path (see LayoutTests/imported/w3c/resources/config.json)
> when starting the WPT server.
> This can be added in the future if that proves to be useful.

This could be done with a few lines of code as follows:

diff --git a/LayoutTests/imported/w3c/resources/config.json b/LayoutTests/imported/w3c/resources/config.json
index ce30f1f..2678782 100644
--- a/LayoutTests/imported/w3c/resources/config.json
+++ b/LayoutTests/imported/w3c/resources/config.json
@@ -9,7 +9,7 @@
          "encrypt_after_connect": false,
          "openssl": {
              "openssl_binary": "openssl",
-             "base_path": "_certs",
+             "base_path": "%CERTS_DIR%",
              "force_regenerate": false
          },
          "pregenerated": {
diff --git a/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py b/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py
index 332c3c3..4445121 100755
--- a/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py
+++ b/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py
@@ -96,7 +96,8 @@ class WebPlatformTestServer(http_server_base.HttpServerBase):
         _log.debug('Copying WebKit web platform server config.json')
         config_wk_filename = self._filesystem.join(self._layout_root, "imported", "w3c", "resources", "config.json")
         if self._filesystem.isfile(config_wk_filename):
-            self._filesystem.copyfile(config_wk_filename, self._filesystem.join(self._doc_root, "config.json"))
+            config_json = self._filesystem.read_text_file(config_wk_filename).replace("%CERTS_DIR%", self._filesystem.join(self._output_dir, "wpt_certs"))
+            self._filesystem.write_text_file(self._filesystem.join(self._doc_root, "config.json"), config_json)
 
     def _clean_webkit_test_files(self):
         _log.debug('Cleaning WPT resources files')
Comment 5 youenn fablet 2015-02-28 13:48:44 PST
> It might be slightly better to generate them in
> WebKitBuild/.../layout-test-results. This may be done by updating the field
> ssl.open_ssl.base_path (see LayoutTests/imported/w3c/resources/config.json)
> when starting the WPT server.
> This can be added in the future if that proves to be useful.

This could be done with a few lines of code:

diff --git a/LayoutTests/imported/w3c/resources/config.json b/LayoutTests/imported/w3c/resources/config.json
index ce30f1f..2678782 100644
--- a/LayoutTests/imported/w3c/resources/config.json
+++ b/LayoutTests/imported/w3c/resources/config.json
@@ -9,7 +9,7 @@
          "encrypt_after_connect": false,
          "openssl": {
              "openssl_binary": "openssl",
-             "base_path": "_certs",
+             "base_path": "%CERTS_DIR%",
              "force_regenerate": false
          },
          "pregenerated": {
diff --git a/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py b/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py
index 332c3c3..4445121 100755
--- a/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py
+++ b/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py
@@ -96,7 +96,8 @@ class WebPlatformTestServer(http_server_base.HttpServerBase):
         _log.debug('Copying WebKit web platform server config.json')
         config_wk_filename = self._filesystem.join(self._layout_root, "imported", "w3c", "resources", "config.json")
         if self._filesystem.isfile(config_wk_filename):
-            self._filesystem.copyfile(config_wk_filename, self._filesystem.join(self._doc_root, "config.json"))
+            config_json = self._filesystem.read_text_file(config_wk_filename).replace("%CERTS_DIR%", self._filesystem.join(self._output_dir, "wpt_certs"))
+            self._filesystem.write_text_file(self._filesystem.join(self._doc_root, "config.json"), config_json)
 
     def _clean_webkit_test_files(self):
         _log.debug('Cleaning WPT resources files')
Comment 6 youenn fablet 2015-02-28 13:56:00 PST
*** Bug 142136 has been marked as a duplicate of this bug. ***
Comment 7 mitz 2015-02-28 14:13:53 PST
(In reply to comment #5)
> > It might be slightly better to generate them in
> > WebKitBuild/.../layout-test-results. This may be done by updating the field
> > ssl.open_ssl.base_path (see LayoutTests/imported/w3c/resources/config.json)
> > when starting the WPT server.
> > This can be added in the future if that proves to be useful.
> 
> This could be done with a few lines of code:

I think we should do this rather than just attachment 247508 [details].
Comment 8 youenn fablet 2015-03-02 09:35:41 PST
(In reply to comment #7)
> (In reply to comment #5)
> > > It might be slightly better to generate them in
> > > WebKitBuild/.../layout-test-results. This may be done by updating the field
> > > ssl.open_ssl.base_path (see LayoutTests/imported/w3c/resources/config.json)
> > > when starting the WPT server.
> > > This can be added in the future if that proves to be useful.
> > 
> > This could be done with a few lines of code:
> 
> I think we should do this rather than just attachment 247508 [details].

Maybe r+/cq+ this patch asap to make developers repo clean.
I can then quickly submit a patch to migrate the certificates location.
Comment 9 Carlos Alberto Lopez Perez 2015-03-03 03:40:38 PST
After a conversation on the IRC, re-assigning to yoenn as I don't have time at this moment for this bug and seems that yoenn already has the right patch in the works to fix this.
Comment 10 Carlos Alberto Lopez Perez 2015-03-03 03:41:27 PST
(In reply to comment #9)
> After a conversation on the IRC, re-assigning to yoenn as I don't have time
> at this moment for this bug and seems that yoenn already has the right patch
> in the works to fix this.

sorry: s/yoenn/youenn/

regards.
Comment 11 youenn fablet 2015-03-03 04:43:50 PST
Created attachment 247755 [details]
Moving _certs location to layout-test-results folder
Comment 12 Simon Fraser (smfr) 2015-03-05 14:37:06 PST
Comment on attachment 247755 [details]
Moving _certs location to layout-test-results folder

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

> Tools/ChangeLog:3
> +        Layout tests imported/w3c/web-platform-tests certificates should not be tracked on the VCS (git)

WebKit's VCS is actually svn.
Comment 13 youenn fablet 2015-03-06 00:20:52 PST
Created attachment 248049 [details]
Patch for landing
Comment 14 WebKit Commit Bot 2015-03-06 00:24:23 PST
Comment on attachment 248049 [details]
Patch for landing

Rejecting attachment 248049 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 248049, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
tests/_certs/serial is not empty after patch, as expected
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/imported/w3c/web-platform-tests/_certs/serial.rej
rm 'LayoutTests/imported/w3c/web-platform-tests/_certs/serial'
patching file LayoutTests/imported/w3c/web-platform-tests/_certs/serial.old
rm 'LayoutTests/imported/w3c/web-platform-tests/_certs/serial.old'

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.appspot.com/results/5942501508644864
Comment 15 youenn fablet 2015-03-06 01:14:37 PST
Created attachment 248053 [details]
Patch for landing
Comment 16 WebKit Commit Bot 2015-03-06 01:27:41 PST
Comment on attachment 248053 [details]
Patch for landing

Clearing flags on attachment: 248053

Committed r181146: <http://trac.webkit.org/changeset/181146>
Comment 17 WebKit Commit Bot 2015-03-06 01:27:47 PST
All reviewed patches have been landed.  Closing bug.
Comment 18 David Kilzer (:ddkilzer) 2015-03-06 09:20:23 PST
(In reply to comment #16)
> Comment on attachment 248053 [details]
> Patch for landing
> 
> Clearing flags on attachment: 248053
> 
> Committed r181146: <http://trac.webkit.org/changeset/181146>

The bots weren't happen since I think these files were modified locally, and now they're being removed (see also Bug 141946, Comment #23):

$ svn stat .
A  +  C LayoutTests/imported/w3c/web-platform-tests/_certs
      >   local edit, incoming delete upon update
C  +    LayoutTests/imported/w3c/web-platform-tests/_certs/01.pem
C  +    LayoutTests/imported/w3c/web-platform-tests/_certs/02.pem
C  +    LayoutTests/imported/w3c/web-platform-tests/_certs/cacert.pem
C  +    LayoutTests/imported/w3c/web-platform-tests/_certs/cakey.pem
C  +    LayoutTests/imported/w3c/web-platform-tests/_certs/index.txt.old
C  +    LayoutTests/imported/w3c/web-platform-tests/_certs/localhost.key
C  +    LayoutTests/imported/w3c/web-platform-tests/_certs/localhost.pem
D     C LayoutTests/imported/w3c/web-platform-tests/tools/scripts/__init__.py
      >   local unversioned, incoming add upon update
D     C LayoutTests/imported/w3c/web-platform-tests/tools/webdriver/webdriver/__init__.py
      >   local unversioned, incoming add upon update
D     C Tools/Scripts/webkitpy/thirdparty/irc
      >   local unversioned, incoming add upon update
D       Tools/Scripts/webkitpy/thirdparty/irc/__init__.py
D       Tools/Scripts/webkitpy/thirdparty/irc/ircbot.py
D       Tools/Scripts/webkitpy/thirdparty/irc/irclib.py
?       layout-test-results.zip
Summary of conflicts:
  Text conflicts: 7
  Tree conflicts: 4
Comment 19 David Kilzer (:ddkilzer) 2015-03-06 09:32:00 PST
(In reply to comment #18)
> (In reply to comment #16)
> > Comment on attachment 248053 [details]
> > Patch for landing
> > 
> > Clearing flags on attachment: 248053
> > 
> > Committed r181146: <http://trac.webkit.org/changeset/181146>
> 
> The bots weren't happen since I think these files were modified locally, and
> now they're being removed (see also Bug 141946, Comment #23):
> 
> $ svn stat .
> A  +  C LayoutTests/imported/w3c/web-platform-tests/_certs
>       >   local edit, incoming delete upon update
> C  +    LayoutTests/imported/w3c/web-platform-tests/_certs/01.pem
> C  +    LayoutTests/imported/w3c/web-platform-tests/_certs/02.pem
> C  +    LayoutTests/imported/w3c/web-platform-tests/_certs/cacert.pem
> C  +    LayoutTests/imported/w3c/web-platform-tests/_certs/cakey.pem
> C  +    LayoutTests/imported/w3c/web-platform-tests/_certs/index.txt.old
> C  +    LayoutTests/imported/w3c/web-platform-tests/_certs/localhost.key
> C  +    LayoutTests/imported/w3c/web-platform-tests/_certs/localhost.pem
> D     C LayoutTests/imported/w3c/web-platform-tests/tools/scripts/__init__.py
>       >   local unversioned, incoming add upon update
> D     C
> LayoutTests/imported/w3c/web-platform-tests/tools/webdriver/webdriver/
> __init__.py
>       >   local unversioned, incoming add upon update
> D     C Tools/Scripts/webkitpy/thirdparty/irc
>       >   local unversioned, incoming add upon update
> D       Tools/Scripts/webkitpy/thirdparty/irc/__init__.py
> D       Tools/Scripts/webkitpy/thirdparty/irc/ircbot.py
> D       Tools/Scripts/webkitpy/thirdparty/irc/irclib.py
> ?       layout-test-results.zip
> Summary of conflicts:
>   Text conflicts: 7
>   Tree conflicts: 4

To clean this up on one bot, I used:

$ svn revert --recursive LayoutTests/imported/w3c/web-platform-tests
Reverted 'LayoutTests/imported/w3c/web-platform-tests/_certs'
Reverted 'LayoutTests/imported/w3c/web-platform-tests/_certs/01.pem'
Reverted 'LayoutTests/imported/w3c/web-platform-tests/_certs/02.pem'
Reverted 'LayoutTests/imported/w3c/web-platform-tests/_certs/cacert.pem'
Reverted 'LayoutTests/imported/w3c/web-platform-tests/_certs/cakey.pem'
Reverted 'LayoutTests/imported/w3c/web-platform-tests/_certs/index.txt.attr'
Reverted 'LayoutTests/imported/w3c/web-platform-tests/_certs/index.txt.attr.old'
Reverted 'LayoutTests/imported/w3c/web-platform-tests/_certs/index.txt.old'
Reverted 'LayoutTests/imported/w3c/web-platform-tests/_certs/localhost.key'
Reverted 'LayoutTests/imported/w3c/web-platform-tests/_certs/localhost.pem'
Reverted 'LayoutTests/imported/w3c/web-platform-tests/_certs/serial'
Reverted 'LayoutTests/imported/w3c/web-platform-tests/_certs/serial.old'
Reverted 'LayoutTests/imported/w3c/web-platform-tests/tools/scripts/__init__.py'
Reverted 'LayoutTests/imported/w3c/web-platform-tests/tools/webdriver/webdriver/__init__.py'

$ svn stat .
?       LayoutTests/imported/w3c/web-platform-tests/_certs
D     C Tools/Scripts/webkitpy/thirdparty/irc
      >   local unversioned, incoming add upon update
D       Tools/Scripts/webkitpy/thirdparty/irc/__init__.py
D       Tools/Scripts/webkitpy/thirdparty/irc/ircbot.py
D       Tools/Scripts/webkitpy/thirdparty/irc/irclib.py
?       layout-test-results.zip
?       local
Summary of conflicts:
  Tree conflicts: 1

However, that leaves the empty _certs directory, so then we have to delete that:

$ rm -rf LayoutTests/imported/w3c/web-platform-tests/_certs
$ svn stat .
D     C Tools/Scripts/webkitpy/thirdparty/irc
      >   local unversioned, incoming add upon update
D       Tools/Scripts/webkitpy/thirdparty/irc/__init__.py
D       Tools/Scripts/webkitpy/thirdparty/irc/ircbot.py
D       Tools/Scripts/webkitpy/thirdparty/irc/irclib.py
?       layout-test-results.zip
?       local
Summary of conflicts:
  Tree conflicts: 1

I'm not sure if there is a better combination of svn commands to use to revert to a known-good state.
Comment 20 David Kilzer (:ddkilzer) 2015-03-06 10:24:53 PST
And:  rm -rf LayoutTests/imported/w3c/web-platform-tests/config.json
Comment 21 youenn fablet 2015-03-06 10:37:34 PST
(In reply to comment #20)
> And:  rm -rf LayoutTests/imported/w3c/web-platform-tests/config.json

Thanks for fixing this mess.
This config.json should not trigger any issue with the bots as it should be overwritten at wpt server start and deleted at shutdown.
Was it causing trouble? Would it help to svn/git ignore it?