In T156039 we enable RelatedArticles for only some subset of users but browser tests do not verify RelatedArticlesEnabledSamplingRate configuration value.
As a result, some browser tests fail because RelatedArticles feature is not visible due to default 0.9 sampling rate (they get bucketed as 0.1 - do not show RelatedArticles feature)
For development purposes you can replicate this by setting $wgRelatedArticlesEnabledSamplingRate to 0 and opting into the beta feature and viewing on desktop.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | ovasileva | T154681 Deploy related pages to mobile web stable for 90% English | |||
| Resolved | Jdlrobson | T157165 Deploying related pages broke our browser tests | |||
| Resolved | ovasileva | T157372 [SUBTASK] RelatedArticlesEnabledSamplingRate breaks RelatedArticles browser tests |
Change 336348 had a related patch set uploaded (by Jdlrobson):
Always show RelatedArticles to users who opted into beta feature
Change 336554 had a related patch set uploaded (by Jdlrobson):
Always show RelatedArticles to users who opted into beta feature
Change 336554 merged by jenkins-bot:
Always show RelatedArticles to users who opted into beta feature
Waitโฆ There's also rERARfa8e34825b5f: Always show RelatedArticles to users who opted into beta featureโฆ But rERAR36dfaccd2fec: Always show RelatedArticles to users who opted into beta feature seems to cover the feedback from @bmansurov and myselfโฆย I'm confusedโฆ
To unconfuse you I can explain what happened here :)
!class_exists( 'BetaFeatures' ) || ( class_exists( 'BetaFeatures' ) && self::isUserOptedIntoBetaFeature( $user ) );
Change 336348 abandoned by Jdlrobson:
Always show RelatedArticles to users who opted into beta feature
Reason:
https://gerrit.wikimedia.org/r/336554 has been merged
Tests #302 through #304 are passing. Strange given the patch has been merged today. (The browser tests have been updated in the parent task.)
I set $wgRelatedArticlesEnabledSamplingRate = 0; locally and observed that I could still see related articles in Vector when I was opted into the beta feature. However, I also was able to see related articles in Minerva, which I should not have been to as the enabled sampling rate was set to 0 and Minerva doesn't care about BetaFeatures. Only when I disabled the beta feature by switching to Vector, I was not able to see related articles in Minerva.
I think this task needs more work as visibility of related articles in Minerva should only depend on the config variable above not and on the value of beta features.
@bmansurov Thanks for pointing out the Minerva situation... yes if you've opted into the beta feature on desktop then you will bypass the bucketing on the mobile site. This wasn't specified and seemed the correct behaviour to me as to bucketing those users would lead to even more technical debt similar to that mentioned in T146436. We'd have to add another if skin == 'minerva' line.
@ovasileva if a logged in user is opted into related pages on the desktop site, they will unconditionally see related pages on the mobile site (even if they are in the 10% of users that shouldn't). Is this a problem we need to fix?
If it is, I recommend we prioritise T146436 for next sprint and fold this new requirement into T157375.
Back to browser tests:
I should point out the browser test builds will still fail 10% of the time if the user is bucketed into the situation where related pages is disabled. This will continue until we drop the 90% roll out from English Wikipedia or update the beta cluster to not respect bucketing. This will add a little noise to our browser tests which I think is fine if it's only expected for a 2 week period.
If we want to SWAT a config change to remove the temporary 10% failure rate we should consider this closed otherwise we should move back to "needs more work".
As for browser tests, can we not set the value of wgRelatedArticlesEnabledSamplingRate to 1 in LocalSettings.php that's inside the browser tests' folder?
Change 336732 had a related patch set uploaded (by Jdlrobson):
Beta cluster should show related pages 100% of time
It defaults to 1. LocalSettings.php is not run on the beta cluster for security reasons. I'll see if I can get the above patch swatted.
Change 336732 merged by jenkins-bot:
Beta cluster should show related pages 100% of time
The change has been SWATed so the beta cluster will now always show related pages. I've done some testing and can verify mw.config.values.wgRelatedArticlesEnabledSamplingRate is always 1 on the beta cluster.
I've tracked the issue with sampling in https://phabricator.wikimedia.org/T157375#3011769 which I consider out of the scope for getting our browser tests passing again.
I think we've knowingly introduced a regression in the behaviour of Related Articles โ for my part, I should've spotted this during code review and testing โ so I agree with @bmansurov that this needs more work.
@ovasileva: To reiterate @Jdlrobson's question:
[If] a logged in user [has] opted into Related Pages on the desktop site, they will unconditionally see related pages on the mobile site (even if they are in the 10% of users that shouldn't). Is this a problem we need to fix?
@ovasileva if a logged in user is opted into related pages on the desktop site, they will unconditionally see related pages on the mobile site (even if they are in the 10% of users that shouldn't). Is this a problem we need to fix?
If it is, I recommend we prioritise T146436 for next sprint and fold this new requirement into T157375.
No, it's such a small subset of users, it will be fine. Question though - will we track these users as "off" in the schema?
Per T157372#3012653.
No sampling will take place on either the desktop or mobile site if the user has enabled the Related Pages beta feature.