Currently, the stub generators don't run on CI. We just use them when we know that packets or stubs would have changed. However, this means that stubs could go stale, and then our unit tests wouldn't catch possible issues when the backend changes. We should set up the tests to run in CI. When the generated packet or stub is different from the one saved in the file, the test would fail with a message that the developer should update the stub files. When they do that, the unit tests will run with the new stubs in CI (Bug 1312823) and fail if it was a breaking change. This has the side benefit of giving us a clear record of all API changes.

okay, so here's a try run which contains the tests, and edits to the code that would change the stubs. If everything goes well, all the check_* tests should fail. https://treeherder.mozilla.org/#/jobs?repo=try&revision=44207742bcbf728148f34411768f49fd5a20c0e9

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #1) > okay, so here's a try run which contains the tests, and edits to the code > that would change the stubs. If everything goes well, all the check_* tests > should fail. > > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=44207742bcbf728148f34411768f49fd5a20c0e9 There were some problems with this, let's try https://treeherder.mozilla.org/#/jobs?repo=try&revision=668d8f1019af11f5255b347517d9233dc6cf1cf9

Okay, now this seems good. This is the try run with my patch : https://treeherder.mozilla.org/#/jobs?repo=try&revision=b218e5850a5f069effe5494e3f25459223fcd0c4 , which show no errors. And here's the one where I made a change which impact the stubs, without updating them : https://treeherder.mozilla.org/#/jobs?repo=try&revision=a35dc336c4c0156df67d0db88c00d195fb8a7cab We can see that the tests fail as expected (e.g. https://treeherder.mozilla.org/#/jobs?repo=try&revision=a35dc336c4c0156df67d0db88c00d195fb8a7cab&selectedJob=78204968 ). I'll push my patch to review soon

Comment on attachment 8838492 [details] Bug 1313688 - Move new console stub generation to head.js; https://reviewboard.mozilla.org/r/111846/#review114994 ::: devtools/client/webconsole/new-console-output/test/fixtures/stub-generators/browser.ini:12 (Diff revision 1) > test-network-event.html > - test-tempfile.css > - test-tempfile.js > > [browser_webconsole_update_stubs_console_api.js] > skip-if=true # This is only used to update stubs. It is not an actual test. not relevant to this changeset, but you should be able to set skip-if = true on the whole test suite instead of disabling each test individually. ::: devtools/client/webconsole/new-console-output/test/fixtures/stub-generators/head.js:88 (Diff revision 1) > res.message.arguments.forEach((argument, i) => { > + let existingArgument = existingPacket.message.arguments[i]; > if (argument && argument.actor) { > - argument.actor = existingPacket.message.arguments[i].actor; > + argument.actor = existingArgument.actor; > + } > + if (argument && argument.class === "Window") { Maybe a comment here explaining why we do this? ::: devtools/client/webconsole/new-console-output/test/fixtures/stub-generators/head.js:228 (Diff revision 1) > }; > `; > } > + > +function* generateConsoleApiStubs() { > + const { consoleApi: snippets } = require("devtools/client/webconsole/new-console-output/test/fixtures/stub-generators/stub-snippets.js"); Maybe we could have a single require for the whole file here?

Comment on attachment 8838492 [details] Bug 1313688 - Move new console stub generation to head.js; https://reviewboard.mozilla.org/r/111846/#review114994 > Maybe we could have a single require for the whole file here? Do you mean moving `const { consoleApi, pageError, ...} = require("devtools/client/webconsole/new-console-output/test/fixtures/stub-generators/stub-snippets.js");` outside of the `generateConsoleApiStubs` function (and the others alike) ?

Comment on attachment 8838492 [details] Bug 1313688 - Move new console stub generation to head.js; https://reviewboard.mozilla.org/r/111846/#review114994 > not relevant to this changeset, but you should be able to set skip-if = true on the whole test suite instead of disabling each test individually. I think we will still need to disable them one by one since we add the check tests here (in another commit)

Comment on attachment 8838493 [details] Bug 1313688 - Stubs generation; https://reviewboard.mozilla.org/r/111848/#review115032

Comment on attachment 8838494 [details] Bug 1313688 - Fix mocha tests. https://reviewboard.mozilla.org/r/113426/#review115060

Comment on attachment 8838495 [details] Bug 1313688 - Fail a test if stubs don't match stubs created by stub generators; https://reviewboard.mozilla.org/r/113428/#review115064 Great improvement, thanks!

Pushed by chevobbe.nicolas@gmail.com: https://hg.mozilla.org/integration/autoland/rev/2add96f767a5 Move new console stub generation to head.js; r=jdescottes https://hg.mozilla.org/integration/autoland/rev/a0e7769a42f3 Stubs generation; r=jdescottes https://hg.mozilla.org/integration/autoland/rev/99ef9fe1494c Fix mocha tests. r=jdescottes https://hg.mozilla.org/integration/autoland/rev/0d7b8fb5255e Fail a test if stubs don't match stubs created by stub generators; r=jdescottes

https://hg.mozilla.org/mozilla-central/rev/2add96f767a5 https://hg.mozilla.org/mozilla-central/rev/a0e7769a42f3 https://hg.mozilla.org/mozilla-central/rev/99ef9fe1494c https://hg.mozilla.org/mozilla-central/rev/0d7b8fb5255e