Page MenuHomePhabricator

Decide if/how to solve wrong link in GE projects to notice soon
Closed, DeclinedPublic

Description

We can probably (browser/integration) test T231935 to not happen again. An engineer can prevent this to happen, but we should decide if we want it to first. Filling this task for MMiller to decide if we want to spend time to implement an automated test to prevent this to happen again, at (any) point in the future.

See this for general comment:

Pinging a few involved people.

@Tgr (merged patch) @Catrope (consulted with him after hotfixing) @kostajh (created the patch that seems to broke this) to investigate further and implement a longer-term solution.

@MMiller_WMF @nettrom_WMF to discuss real impact and how important mitigations to prevent this in the future are.

And a screenshot of the issue:

image.png (661×634 px, 49 KB)

Assigning to @MMiller_WMF, and setting to PM review needed, not 100% sure through.

Event Timeline

@Urbanecm - the work for automated tests for Homepage has started: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/GrowthExperiments/+/517200/. We should prioritize automation testing work to increase the test coverage.

@Urbanecm -- I'm not sure what this task is and how I can help. Could you please explain some more?

@MMiller_WMF Sure. In T231935, we ran into a case when a "Site help" link was displayed despite it wasn't in the configuration. There are ways how to test this automatically, so each patch that would (theoretically) break that again would be rejected right away. I wanted to propose to add said test, so this wouldn't happen again.

@MMiller_WMF Sure. In T231935, we ran into a case when a "Site help" link was displayed despite it wasn't in the configuration. There are ways how to test this automatically, so each patch that would (theoretically) break that again would be rejected right away. I wanted to propose to add said test, so this wouldn't happen again.

Do you have thoughts on how we'd test specifically this instance? The bug occurred because of unanticipated results from merging two configuration arrays. We currently can't have a CI specific configuration (see T199939), so it would be hard to reproduce this problem there. Maybe we could do a Selenium daily beta cluster job (e.g. https://integration.wikimedia.org/ci/job/selenium-daily-beta-MobileFrontend/) but from what I've seen, these tend to fail quite often due to beta cluster breakages, so then we'd be ignoring the output. Might be better than nothing though.

I am moving this task to "Needs Discussion", since it's not clear what we would be doing here.

The Growth-Team believes it is not worth writing an automated test for this and we will have to update it every time we update the configuration so it will be more annoying than helpful. The previous bug is also a pretty idiosyncratic issue.