Note: One possible alternative to updating the file would be to check whether the file is updated and give a warning and suggested command to run, if it's not. (This might be a little more complex though, since it would probably involve updating the manifest, and then undoing if there are any changes.)

This would be consistent with some other behavior of some other presubmits, e.g. the formatting check (https://cs.chromium.org/chromium/tools/depot_tools/presubmit_canned_checks.py?l=1128) which produces a warning with a suggestion "... Please run git cl format third_party/WebKit", but this warning can be skipped or ignored.

Does that sound like a nicer behavior?

TBH the alternative in #1 sounds a tad worse to me.

IMHO it makes sense that source code changes made by "git cl format" require some manual interaction, because it impacts content that developers interact with (e.g. during reviews). After running git cl format, I'd want to see the result, as it might prompt me to make code changes.

By contrast, MANIFEST.json is a build product that happens to be checked into our repository for performance reasons. In an ideal world, it'd be completely invisible to developers. Given that the diffs will show up in code review, I think the presubmit should just announce that it changed MANIFEST.json (when applicable). There's nothing actionable there -- I'll never want to read the file, or to tweak it.

I hope this helps.

Until this happens... MANIFEST.json seems to have diverged from the repository content. Regenerating it for a CL I'm working on gives me the diff below.

Should I let this diff creep into my CL, or try to only check the diff hunks related to the tests I'm adding? The former would cause a bit of reviewer confusion, while the latter would be a minor pain every time I add tests.

Thank you!

@@ -4874,6 +4879,11 @@
      {}
     ]
    ],
+   "domparsing/innerhtml-05-expected.txt": [
+    [
+     {}
+    ]
+   ],
    "domparsing/insert_adjacent_html-expected.txt": [
     [
      {}
@@ -6239,6 +6249,16 @@
      {}
     ]
    ],
+   "html/browsers/history/the-location-interface/location-protocol-setter-non-broken-expected.txt": [
+    [
+     {}
+    ]
+   ],
+   "html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird-expected.txt": [
+    [
+     {}
+    ]
+   ],
    "html/browsers/history/the-location-interface/location-prototype-setting-expected.txt": [
     [
      {}
@@ -12324,11 +12344,6 @@
      {}
     ]
    ],
-   "html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/not-in-shadow-tree-expected.txt": [
-    [
-     {}
-    ]
-   ],
    "html/semantics/document-metadata/the-style-element/html_style_in_comment-expected.html": [
     [
      {}
@@ -12844,11 +12859,6 @@
      {}
     ]
    ],
-   "html/semantics/forms/the-fieldset-element/disabled-001-expected.txt": [
-    [
-     {}
-    ]
-   ],
    "html/semantics/forms/the-form-element/form-autocomplete-expected.txt": [
     [
      {}
@@ -13894,6 +13904,21 @@
      {}
     ]
    ],
+   "html/syntax/parsing/html5lib_innerHTML_adoption01-expected.txt": [
+    [
+     {}
+    ]
+   ],
+   "html/syntax/parsing/html5lib_innerHTML_foreign-fragment-expected.txt": [
+    [
+     {}
+    ]
+   ],
+   "html/syntax/parsing/html5lib_innerHTML_webkit02-expected.txt": [
+    [
+     {}
+    ]
+   ],
    "html/syntax/parsing/html5lib_isindex-expected.txt": [
     [
      {}
@@ -34758,6 +34817,10 @@
    "7becf54b03320a3b905c4ebfa476476f22409307",
    "testharness"
   ],
+  "IndexedDB/idbcursor-continue-exception-order.htm": [
+   "5c6bd64850880ceb20639325cf3d61127e2b13c8",
+   "testharness"
+  ],
   "IndexedDB/idbcursor-continue.htm": [
    "953ae4b03fcfe2be3652df09577df2e3829dd53a",
    "testharness"
@@ -34770,6 +34833,10 @@
    "7c2ddb95ce4b7c588ddb2907367a365902eba349",
    "testharness"
   ],
+  "IndexedDB/idbcursor-delete-exception-order.htm": [
+   "6bb279530dbdedba84a2d45d58e5d0a81a4d95e5",
+   "testharness"
+  ],
   "IndexedDB/idbcursor-direction-index-keyrange.htm": [
    "d87cc37174c74e57880f8883e75a9c033c021e74",
    "testharness"
@@ -34806,6 +34873,10 @@
    "0677de106509b9b3f4d1e38780f479d9aaace626",
    "testharness"
   ],
+  "IndexedDB/idbcursor-update-exception-order.htm": [
+   "748c1764883cc62e58c1ce7929e91e6ee2941fda",
+   "testharness"
+  ],
   "IndexedDB/idbcursor_advance_index.htm": [
    "b6cb5fd53000be9c8d198b08d259a1062a8f9946",
    "testharness"
@@ -35046,6 +35117,14 @@
    "7534a0868a5861cfd9f2fef0f3eddf452c0d9366",
    "testharness"
   ],
+  "IndexedDB/idbdatabase-createObjectStore-exception-order.htm": [
+   "2a8e730d2e7985942e4722fc38769d7d10b79d01",
+   "testharness"
+  ],
+  "IndexedDB/idbdatabase-deleteObjectStore-exception-order.htm": [
+   "89806059c60621b1602b0d91343f7e25a2eb3124",
+   "testharness"
+  ],
   "IndexedDB/idbdatabase_close.htm": [
    "6b0a32c465f735544b89da588bb043bbfbb66230",
    "testharness"
@@ -36031,7 +36110,7 @@
    "support"
   ],
   "common/vendor-prefix.js": [
-   "2807645fda3cb1decbb944627b038fc87ef019a0",
+   "b80ec842b65a3423dd789a2226f2ba3caf598049",
    "support"
   ],
   "css-values/unset-value-storage.html": [
@@ -37946,6 +38025,10 @@
    "c543183d2f6b636a1e64a593f00599046a6aa632",
    "testharness"
   ],
+  "domparsing/innerhtml-05-expected.txt": [
+   "11b3f513b7cc424896e5854ccfd6950d7da6940f",
+   "support"
+  ],
   "domparsing/innerhtml-05.xhtml": [
    "3b31aaee88263c1458f5472adc52ab47a5a9ec29",
    "testharness"
@@ -40214,6 +40297,14 @@
    "011522f6b56184f49dd6555b507d6d895b53f11e",
    "testharness"
   ],
+  "html/browsers/history/the-location-interface/location-protocol-setter-non-broken-expected.txt": [
+   "dba0da022025260daffcf99f99ee4d65bfaa6dfe",
+   "support"
+  ],
+  "html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird-expected.txt": [
+   "60f2755719f6074c3f6e67e4fe140358acf6093c",
+   "support"
+  ],
   "html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird.html": [
    "13a360b3010a85c1661d9357355894453137c9df",
    "testharness"
@@ -40483,7 +40574,7 @@
    "support"
   ],
   "html/browsers/origin/cross-origin-objects/cross-origin-objects-exceptions-expected.txt": [
-   "56bb3aabf5fe7ba9d38e088224ad3b7ee60d8c3a",
+   "5074e545dc1000b55718c5c6bd9de0a7b6832a9c",
    "support"
   ],
   "html/browsers/origin/cross-origin-objects/cross-origin-objects-exceptions.html": [
@@ -47182,10 +47273,6 @@
    "050190b2706ff41b0a5844ca159990908d57177c",
    "testharness"
   ],
-  "html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/not-in-shadow-tree-expected.txt": [
-   "fd72553cfc141995ec78c5f566976d8dc32da5fa",
-   "support"
-  ],
   "html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/not-in-shadow-tree.html": [
    "0d5f568af3b295b36e390156caf5ce523fd83d93",
    "testharness"
@@ -47251,7 +47338,7 @@
    "support"
   ],
   "html/semantics/embedded-content/image-maps/image-map-processing-model/hash-name-reference-expected.txt": [
-   "ea9c8663f79e6019fcd291be5e66ed6b25dd64ec",
+   "eec10c1f9afcedeaf7a89648d073fc0bb9795560",
    "support"
   ],
   "html/semantics/embedded-content/image-maps/image-map-processing-model/hash-name-reference-test-data.html": [
@@ -48023,7 +48110,7 @@
    "testharness"
   ],
   "html/semantics/embedded-content/the-img-element/usemap-casing-expected.txt": [
-   "cefce32704068e2e9a744727cea15d777276df73",
+   "ea5151cf6e1d2ca4d704afd73e3060a8d1875b84",
    "support"
   ],
   "html/semantics/embedded-content/the-img-element/usemap-casing.html": [
@@ -48334,10 +48421,6 @@
    "7d6b7eb6d9daa95a09469181e8c62c8174be5d52",
    "testharness"
   ],
-  "html/semantics/forms/the-fieldset-element/disabled-001-expected.txt": [
-   "c9ee727c5101c14d586d41e8c61da9f2a2aaa8e1",
-   "support"
-  ],
   "html/semantics/forms/the-fieldset-element/disabled-001.html": [
    "f8c2eeac272c802f70ee5e520eab3212c016604f",
    "testharness"
@@ -49167,7 +49250,7 @@
    "testharness"
   ],
   "html/semantics/interfaces-expected.txt": [
-   "c07c7a890f8d7ba9a626d7b63de29ac11691b79f",
+   "0370383eb0419d761e112e984ba8e7246ea82902",
    "support"
   ],
   "html/semantics/interfaces.html": [
@@ -50294,10 +50377,18 @@
    "1e8001486b51c6c37c6d9ae5ccb2540f8ff7676f",
    "testharness"
+     "/IndexedDB/idbdatabase-deleteObjectStore-exception-order.htm",
+     {}
+    ]
+   ],
    "IndexedDB/idbdatabase_close.htm": [
     [
      "/IndexedDB/idbdatabase_close.htm",
@@ -34506,6 +34561,10 @@
    "89af73c41d456ee93bd40921e96414c1908fc48c",
    "support"
   ],
+  "./MANIFEST.json": [
+   "b10cfc1d94d587a0d2f9447d0d438e81ef1682dd",
+   "support"
+  ],
   "./README.md": [
    "e9590560efb0d37ab870037edaafc1c47dc7a785",
    "support"
@@ -34758,6 +34817,10 @@
    "7becf54b03320a3b905c4ebfa476476f22409307",
    "testharness"
   ],
+  "IndexedDB/idbcursor-continue-exception-order.htm": [
+   "5c6bd64850880ceb20639325cf3d61127e2b13c8",
+   "testharness"
+  ],
   "IndexedDB/idbcursor-continue.htm": [
    "953ae4b03fcfe2be3652df09577df2e3829dd53a",
    "testharness"
@@ -34770,6 +34833,10 @@
    "7c2ddb95ce4b7c588ddb2907367a365902eba349",
    "testharness"
   ],
+  "IndexedDB/idbcursor-delete-exception-order.htm": [
+   "6bb279530dbdedba84a2d45d58e5d0a81a4d95e5",
+   "testharness"
+  ],
   "IndexedDB/idbcursor-direction-index-keyrange.htm": [
    "d87cc37174c74e57880f8883e75a9c033c021e74",
    "testharness"
@@ -34806,6 +34873,10 @@
    "0677de106509b9b3f4d1e38780f479d9aaace626",
    "testharness"
   ],
+  "IndexedDB/idbcursor-update-exception-order.htm": [
+   "748c1764883cc62e58c1ce7929e91e6ee2941fda",
+   "testharness"
+  ],
   "IndexedDB/idbcursor_advance_index.htm": [
    "b6cb5fd53000be9c8d198b08d259a1062a8f9946",
    "testharness"
@@ -35046,6 +35117,14 @@
    "7534a0868a5861cfd9f2fef0f3eddf452c0d9366",
    "testharness"
   ],
+  "IndexedDB/idbdatabase-createObjectStore-exception-order.htm": [
+   "2a8e730d2e7985942e4722fc38769d7d10b79d01",
+   "testharness"
+  ],
+  "IndexedDB/idbdatabase-deleteObjectStore-exception-order.htm": [
+   "89806059c60621b1602b0d91343f7e25a2eb3124",
+   "testharness"
+  ],
   "IndexedDB/idbdatabase_close.htm": [
    "6b0a32c465f735544b89da588bb043bbfbb66230",
    "testharness"
@@ -36031,7 +36110,7 @@
    "support"
   ],
   "common/vendor-prefix.js": [
-   "2807645fda3cb1decbb944627b038fc87ef019a0",
+   "b80ec842b65a3423dd789a2226f2ba3caf598049",
    "support"
   ],
   "css-values/unset-value-storage.html": [
@@ -37946,6 +38025,10 @@
    "c543183d2f6b636a1e64a593f00599046a6aa632",
    "testharness"
   ],
+  "domparsing/innerhtml-05-expected.txt": [
+   "11b3f513b7cc424896e5854ccfd6950d7da6940f",
+   "support"
+  ],
   "domparsing/innerhtml-05.xhtml": [
    "3b31aaee88263c1458f5472adc52ab47a5a9ec29",
    "testharness"
@@ -40214,6 +40297,14 @@
    "011522f6b56184f49dd6555b507d6d895b53f11e",
    "testharness"
   ],
+  "html/browsers/history/the-location-interface/location-protocol-setter-non-broken-expected.txt": [
+   "dba0da022025260daffcf99f99ee4d65bfaa6dfe",
+   "support"
+  ],
+  "html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird-expected.txt": [
+   "60f2755719f6074c3f6e67e4fe140358acf6093c",
+   "support"
+  ],
   "html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird.html": [
    "13a360b3010a85c1661d9357355894453137c9df",
    "testharness"
@@ -40483,7 +40574,7 @@
    "support"
   ],
   "html/browsers/origin/cross-origin-objects/cross-origin-objects-exceptions-expected.txt": [
-   "56bb3aabf5fe7ba9d38e088224ad3b7ee60d8c3a",
+   "5074e545dc1000b55718c5c6bd9de0a7b6832a9c",
    "support"
   ],
   "html/browsers/origin/cross-origin-objects/cross-origin-objects-exceptions.html": [
@@ -47182,10 +47273,6 @@
    "050190b2706ff41b0a5844ca159990908d57177c",
    "testharness"
   ],
-  "html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/not-in-shadow-tree-expected.txt": [
-   "fd72553cfc141995ec78c5f566976d8dc32da5fa",
-   "support"
-  ],
   "html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/not-in-shadow-tree.html": [
    "0d5f568af3b295b36e390156caf5ce523fd83d93",
    "testharness"
@@ -47251,7 +47338,7 @@
    "support"
   ],
   "html/semantics/embedded-content/image-maps/image-map-processing-model/hash-name-reference-expected.txt": [
-   "ea9c8663f79e6019fcd291be5e66ed6b25dd64ec",
+   "eec10c1f9afcedeaf7a89648d073fc0bb9795560",
    "support"
   ],
   "html/semantics/embedded-content/image-maps/image-map-processing-model/hash-name-reference-test-data.html": [
@@ -48023,7 +48110,7 @@
    "testharness"
   ],
   "html/semantics/embedded-content/the-img-element/usemap-casing-expected.txt": [
-   "cefce32704068e2e9a744727cea15d777276df73",
+   "ea5151cf6e1d2ca4d704afd73e3060a8d1875b84",
    "support"
   ],
   "html/semantics/embedded-content/the-img-element/usemap-casing.html": [
@@ -48334,10 +48421,6 @@
    "7d6b7eb6d9daa95a09469181e8c62c8174be5d52",
    "testharness"
   ],
-  "html/semantics/forms/the-fieldset-element/disabled-001-expected.txt": [
-   "c9ee727c5101c14d586d41e8c61da9f2a2aaa8e1",
-   "support"
-  ],
   "html/semantics/forms/the-fieldset-element/disabled-001.html": [
    "f8c2eeac272c802f70ee5e520eab3212c016604f",
    "testharness"
@@ -49167,7 +49250,7 @@
    "testharness"
   ],
   "html/semantics/interfaces-expected.txt": [
-   "c07c7a890f8d7ba9a626d7b63de29ac11691b79f",
+   "0370383eb0419d761e112e984ba8e7246ea82902",
    "support"
   ],
   "html/semantics/interfaces.html": [
@@ -50294,10 +50377,18 @@
    "1e8001486b51c6c37c6d9ae5ccb2540f8ff7676f",
    "testharness"

It sounds like the best course of action is to make a quick CL to fix the current difference and land it now.

But we should also try to understand why this is diverging in the first place. It looks like most of the changes are to test expectations. There might be something different in the way the WPT importer generates the MANIFEST vs. the behavior of the bare command.

After talking with Quinten it looks like it is expected that -expected.txt files should be showing up in your diff. Usually the WPT importer would incorporate those changes into its update.

Victor - how disruptive would it be to commit those changes along with your CL? If it would create confusion, I can make a quick CL.

More generally, in addition to adding MANIFEST.json regeneration to run-webkit-tests ( bug 666957 ) and during PRESUBMIT (this  bug 684230 ), we should look into adding MANIFEST.json regeneration to whatever updates the test expectations so that MANIFEST.json doesn't slowly creep away from being an accurate representation of the filesystem.

I can commit the diff with the rest of my CL. My reviewer is super-supportive, so I will point here to explain why the diff has unrelated hunks.

There's discussion in  bug 666957  about both of these issues. Blocking this on that bug until we have a clearer picture.

A better solution was done for  bug 666957 ; a manifest base file is checked in, which is updated on import, and MANIFEST.json is generated based on that when tests are run, but ignored by git due to the .gitignore file.