If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

--manifest-update does not work properly on Windows

RESOLVED FIXED

Status

Testing
web-platform-tests
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: xidorn, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

11.54 KB, application/gzip
Details
(Reporter)

Description

2 years ago
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 '\\'
(Reporter)

Comment 1

2 years ago
Created attachment 8684709 [details]
diff

This is the diff mercurial generates after I run "mach web-platform-tests --manifest-update". (The file is too large, so gzipped.)
https://github.com/w3c/wpt-tools/pull/39
(Reporter)

Comment 3

2 years ago
https://github.com/w3c/wpt-tools/pull/41
(Reporter)

Updated

2 years ago
Depends on: 1225909
(Reporter)

Comment 4

2 years ago
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"
           }
         ],
(Reporter)

Comment 5

2 years ago
I also wonder why it generates a very long list in "local_changes" in testing/web-platform/meta/MANIFEST.json
(Reporter)

Comment 6

2 years ago
Created attachment 8690720 [details]
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.
(Reporter)

Comment 8

2 years ago
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.
(Reporter)

Comment 9

2 years ago
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)
(Reporter)

Comment 11

2 years ago
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
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.