Page MenuHomePhabricator

Popups daily jobs currently unusable
Closed, ResolvedPublic


Historically we've relied heavily on daily jobs for the Popups and RelatedArticles extension and they were listed here:

Recently these have disappeared and I assume replaced with and which are failing consistently.

Note, we have absolutely no coverage for detecting bugs in these extensions while these jobs fail (as we don't commit patches to these extensions and they are not part of the core test suite) and it is extremely important in Popups in particular that we keep the service bug free.
Can we please restore the old ones or is there some other stable job we can use in the mean time?

Event Timeline

Jdlrobson created this task.Sep 5 2018, 6:05 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 5 2018, 6:05 PM
Jdlrobson triaged this task as High priority.Sep 5 2018, 6:06 PM

Having no integration tests running for Popups is not acceptable IMO. We rely on these to ensure that Popups is safe for the train at all times, so I'm hoping I'm just overlooking something here.

Jdlrobson renamed this task from Popups and RelatedArticles daily jobs absent to Popups and RelatedArticles daily jobs absent/unusable.Sep 5 2018, 6:06 PM
Jdlrobson moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.
hashar added subscribers: zeljkofilipin, hashar.

The jobs are being migrated off of Nodepool toward Docker container. That is T188742

Popups fails, it was pending a patch in mediawiki/core which meanwhile has been abandoned Zeljko will no more.

The RelatedArticles tests pass all fine, but this morning the job learned to process JUnit result files emitted by wdio. Turns out that when all tests are passing there is no test reports available and the job fails. That was introduced by and I reverted it.

For , the view manually selects jobs. I have added the new ones for Popups and RelatedArticles.

Jdlrobson added a comment.EditedSep 5 2018, 6:38 PM

Thanks for fixing RelatedArticles and the reassurance!

I'm still very nervous about the Popups one as we depend on that a lot. Let me know how I can get that passing again.

Edit: When these new jobs fail I can't seem to see why without jumping into the console output. Is there a way to print this in the failure itself in a similar way to the way we do so for Ruby tests?

hashar added a comment.Sep 5 2018, 7:44 PM

Zeljko and I have a pairing session on Thursday, we will polish up the Popups job and complete the migration :]

zeljkofilipin moved this task from Backlog 🔙 to In Progress 🔨 on the User-zeljkofilipin board.

The only remaining thing to do is to review and merge 449485. As soon as it's merged, Popups job will be fixed.

I'm unable to make a commit in Popups.

$ git status
On branch T203591
Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

	modified:   package-lock.json
	modified:   package.json
$ git diff --cached
diff --git a/package-lock.json b/package-lock.json
index d2dbebf..c6326e0 100644
Binary files a/package-lock.json and b/package-lock.json differ
diff --git a/package.json b/package.json
index e1e6784..1587f3d 100644
--- a/package.json
+++ b/package.json
@@ -41,6 +41,7 @@
     "stylelint-config-wikimedia": "0.4.3",
     "svg-inline-loader": "0.8.0",
     "tap-mocha-reporter": "3.0.7",
+    "wdio-junit-reporter": "0.4.4",
     "wdio-mediawiki": "0.2.0",
     "wdio-mocha-framework": "0.6.1",
     "wdio-spec-reporter": "0.1.4",
$ git commit

> husky - npm run -s precommit

(node:5660) DeprecationWarning: loaderUtils.parseQuery() received a non-string value which can be problematic, see
parseQuery() will be replaced with getOptions() in the next major version of loader-utils.
Hash: 0f94def25715810b4fe7
Version: webpack 4.1.1
Time: 982ms
Built at: 9/6/2018 11:27:31 AM
        Asset      Size  Chunks             Chunk Names
     index.js  36.7 KiB       0  [emitted]  index
index.js.json   181 KiB       0  [emitted]  index
Entrypoint index = index.js index.js.json
[./src/counts.js] 1.54 KiB {0} [built]
[./src/index.js] ./src/index.js + 46 modules 112 KiB {0} [built]
       | ./src/gateway/index.js 1.85 KiB [built]
       | ./src/index.js 7.48 KiB [built]
       | ./src/getPageviewTracker.js 3.25 KiB [built]
       | ./src/integrations/mwpopups.js 458 bytes [built]
       | ./src/reducers/index.js 307 bytes [built]
       | ./src/actions.js 10.7 KiB [built]
       | ./src/changeListeners/index.js 515 bytes [built]
       | ./src/experiments.js 1.47 KiB [built]
       | ./src/userSettings.js 2.42 KiB [built]
       | ./src/previewBehavior.js 1.67 KiB [built]
       | ./src/instrumentation/eventLogging.js 913 bytes [built]
       | ./src/instrumentation/statsv.js 701 bytes [built]
       | ./src/ui/renderer.js 17.5 KiB [built]
       | ./src/ui/settingsDialogRenderer.js 3.72 KiB [built]
       | ./src/changeListener.js 1.24 KiB [built]
       |     + 32 hidden modules
[./src/ui/pointer-mask.svg] 640 bytes {0} [built]
    + 2 hidden modules
diff --git a/resources/dist/index.js b/resources/dist/index.js
index 80895dd..7f76b84 100644
Binary files a/resources/dist/index.js and b/resources/dist/index.js differ
diff --git a/resources/dist/index.js.json b/resources/dist/index.js.json
index ab577b5..6700189 100644
Binary files a/resources/dist/index.js.json and b/resources/dist/index.js.json differ
Please ensure you are running the correct version of nvm before running this command.

> husky - pre-commit hook failed (add --no-verify to bypass)
> husky - to debug, use 'npm run precommit'

@Niedzielski looks like you did NVM related changes to Popups recently, can you help?

Change 458509 had a related patch set uploaded (by Hashar; owner: Hashar):
[mediawiki/extensions/Popups@master] QA: add missing wdio-junit-reporter

Binary files a/resources/dist/index.js.json and b/resources/dist/index.js.json differ

@zeljkofilipin, are you using either nvm or Node v6.11.0 directly? node --version should answer the question. If you have NVM / Node.js configured correctly, you should be able to npm run build master and show a clean git status. If you've made changes recently to your NVM / Node.js configuration, try deleting the node_modules directory in Popups and do a new npm install and npm run build.

Change 458509 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] QA: add missing wdio-junit-reporter

Jdlrobson renamed this task from Popups and RelatedArticles daily jobs absent/unusable to Popups daily jobs currently unusable.Sep 6 2018, 3:51 PM

@zeljkofilipin, are you using either nvm or Node v6.11.0 directly? node --version should answer the question. I

The error message above shows that Zeljko is using Node v10.9.0 so yeh this is likely the issue. We should improve this error message since others are likely to run into it.

Jdlrobson closed this task as Resolved.Sep 6 2018, 5:05 PM

w00t! Thank you both for fixing this so promptly and they are both green which relieves me :)

hashar added a comment.Sep 6 2018, 9:46 PM

I have edited the package.json and ran npm install then have send the result. Locally the package-lock.json was left untouched. I have:


I assume the package-lock.json is for webpack and since the new dependency "wdio-junit-reporter" is not listed for webpack the lock file is not touched?

I don't think package-lock.json works with this version of npm/node (I think npm5 was when it arrive) @Niedzielski mentioned the other day we should probably get rid of it.

If newer versions of Node.js cannot be used to build code, I don't think there's any use for it :|

I'm not sure what the problem is. Are you saying I can't make a commit to Popups unless I have Node v6.11.0? If that is the case, it should be documented in ALL CAPS in the readme. 😉

I'm not sure what the problem is. Are you saying I can't make a commit to Popups unless I have Node v6.11.0? If that is the case, it should be documented in ALL CAPS in the readme. 😉

T202748 would be a better place to have this conversation.