Closed Bug 1222850 Opened 6 years ago Closed 6 years ago

--manifest-update does not work properly on Windows

Categories

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

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: xidorn, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

11.54 KB, application/gzip
Details
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 '\\'
Attached file diff (obsolete) —
This is the diff mercurial generates after I run "mach web-platform-tests --manifest-update". (The file is too large, so gzipped.)
Depends on: 1225909
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"
           }
         ],
I also wonder why it generates a very long list in "local_changes" in testing/web-platform/meta/MANIFEST.json
Attached file 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.
Attachment #8684709 - Attachment is obsolete: true
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.
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.
Could you update the web-platform-tests again so that we can close this issue?
Depends on: 1230948
Flags: needinfo?(james)
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.
Flags: needinfo?(james)
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
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.