1222850 - --manifest-update does not work properly on Windows

Status: RESOLVED FIXED

(Testing :: web-platform-tests, defect)

Description

[Xidorn Quan [:xidorn] UTC+11]

Calling `mach web-platform-tests --manifest-update" on Windows would change the content of the whole manifests into Windows-specific format. There are at least two issue as far as I can see:
1. it replaces all UNIX format newlines with the Windows one
2. it replaces the path slashes from '/' to '\\'

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

Attachment: diff

This is the diff mercurial generates after I run "mach web-platform-tests --manifest-update". (The file is too large, so gzipped.)

Comment 2

[James Graham [:jgraham]]

https://github.com/w3c/wpt-tools/pull/39

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

https://github.com/w3c/wpt-tools/pull/41

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

It seems with the latest wpt, some pathes in testing/web-platform/mozilla/meta/MANIFEST.json still have wrong path separator:

   "local_changes": {
     "deleted": [],
     "items": {
       "testharness": {
-        "service-workers/service-worker/ServiceWorkerGlobalScope/close.https.html": [
+        "service-workers\\service-worker\\ServiceWorkerGlobalScope\\close.https.html": [
           {
             "path": "service-workers/service-worker/ServiceWorkerGlobalScope/close.https.html",
             "url": "/_mozilla/service-workers/service-worker/ServiceWorkerGlobalScope/close.https.html"
           }
         ],

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

I also wonder why it generates a very long list in "local_changes" in testing/web-platform/meta/MANIFEST.json

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

Attachment: new diff

With the current tree and the pull request in comment 3, when I run manifest-update without touching files, it now generates this diff. It has been much better than before, but there is still difference.

Comment 7

[James Graham [:jgraham]]

OK, I'll take another look at this. I didn't actually have Windows on hand to test my patch so it's quite likely I missed something.

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

https://github.com/w3c/wpt-tools/pull/43

With this pull request, there remains only one extraordinary difference, which seems to be a case sensitive issue:
>    "local_changes": {
> -    "deleted": [],
> -    "items": {},
> +    "deleted": [
> +      "html/semantics/grouping-content/the-li-element/grouping-li-novalue-manual.html"
> +    ],
> +    "items": {
> +      "testharness": {
> +        "html/semantics/grouping-content/the-li-element/grouping-li-novalue-MANUAL.html": [
> +          {
> +            "path": "html/semantics/grouping-content/the-li-element/grouping-li-novalue-MANUAL.html",
> +            "url": "/html/semantics/grouping-content/the-li-element/grouping-li-novalue-MANUAL.html"
> +          }
> +        ]
> +      }
> +    },
>      "reftest_nodes": {}
>    },

I guess that word was uppercase when the test was created, and lowercased later, but Mercurial doesn't track the case change properly which causes this issue. Renaming the file manually could fix it without troubling Mercurial, so it's fine. But I guess in long-term we probably want to compare paths case-insensitively, given Windows and OS X both use case-insensitive file system by default.

Comment 9

[Xidorn Quan [:xidorn] UTC+11]

Could you update the web-platform-tests again so that we can close this issue?

Comment 10

[James Graham [:jgraham]]

Yeah, I'm just waiting for a couple of local patches for unstable tests to land and then I will do the update. Tomorrow, I expect.

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

It seems all changes there have been merged, so I'm going to close this bug now.

Note that I found two newly added tests (from bug 1222464 part 3) were in wrong order. I fixed them in https://hg.mozilla.org/integration/mozilla-inbound/rev/60c74d42f521