Don't serve ab tests set to off #14684
Conversation
github-actions
bot
removed
the
run_chromatic
Jakeii
force-pushed
the
jlk/dont-serve-tests-set-to-off
branch
from
6eaf0d3 to
165b25f
Compare
Jakeii
force-pushed
the
jlk/dont-serve-tests-set-to-off
branch
from
165b25f to
fe93951
Compare
github-actions
bot
removed
the
run_chromatic
| export const ABTests: ABTest[] = []; | ||
| const ABTests: ABTest[] = []; | ||
|
|
||
| const activeABtests = ABTests.filter((test) => test.status === 'ON'); |
There was a problem hiding this comment.
Non-blocking low-priority possibly-daft question for my own curiosity: Should we also filter out tests that have expired at this point?
There was a problem hiding this comment.
No thats a sensible question! Expired tests are already ignored by Fastly. They also still currently fail the AB test validation (and CI).
Will address stopping them from breaking the validation (as we discussed in the WebEx catchup), separately! Which will likely involve filtering them out here 👍
There was a problem hiding this comment.
Sorry Monday brain, I added a prompt to the expiration error, https://github.com/guardian/dotcom-rendering/pull/14684/files#diff-4202f37f98d009ed152e65d2bb1a06b05cc758b7b4857bb77628fbdb058b2623L15
The idea is that if you encounter someone else's expired test while adding your own, you can simply set it to off. Hopefully will encourage informing the team!
And we can also hopefully later add a mechanism to alert teams to their expire tests.
There was a problem hiding this comment.
Thanks @Jakeii, I thought Fastly might be filtering out the expired tests. I guess I was thinking, we're doing some filtering here and some filtering on Fastly... why not do it all here? I haven't thought about it too deeply though.
There was a problem hiding this comment.
Ah yes, didn't answer your question did I!
Only on tests are validated, we can't exclude expired tests as we still want them to be validated.
There was a problem hiding this comment.
Oh yes, that makes sense. So the owners of expired tests can receive some sort of notification that the test has indeed expired. Thanks for connecting the dots for me! 😊
* Don't serve ab tests set to off * make test more explicit * don't upload inactive tests to fastly * remove expiry method * metrics * frontend
What does this change?
Exclude tests set to off from being validated and deployed.
Why?
Tests that are turned off should not be served.