Skip to content

Updated labs tests to not rely on specific flags#26622

Open
troyciesco wants to merge 1 commit intomainfrom
update-GA-labs-test
Open

Updated labs tests to not rely on specific flags#26622
troyciesco wants to merge 1 commit intomainfrom
update-GA-labs-test

Conversation

@troyciesco
Copy link
Contributor

@troyciesco troyciesco commented Feb 26, 2026

no ref

  • a test for labs ensures that config values take precedence over GA keys. However, the test referenced the announcementBar flag was removed in Removed announcementBar feature flag #26196. That means this test was looking at a state that wasn't really true
  • instead, we can check for the first item in the GA_KEYS list. If there's nothing in the list, the test can be skipped.
  • a few other tests had this same issue, so this PR updates them as well
  • there was a test for alpha flags that wasn't actually doing anything, so it's deleted

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2082691 and f67ad87.

📒 Files selected for processing (1)
  • ghost/core/test/unit/shared/labs.test.js

Walkthrough

The test file for labs functionality was updated to use dynamic keys instead of hardcoded values. Tests were refactored to verify config precedence over settings using runtime-computed writable flags or GA keys. Skip conditions were added to prevent execution when relevant allowlists or GA keys are empty. Assertions were adjusted to reference the dynamically selected keys in both config injections and expected results. The changes generalize the test scope to validate behavior across any applicable keys rather than a single fixed flag.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: updating labs tests to use dynamic keys instead of hardcoded flags.
Description check ✅ Passed The description is directly related to the changeset, explaining why tests were updated and what the new approach accomplishes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-GA-labs-test

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@troyciesco troyciesco force-pushed the update-GA-labs-test branch 2 times, most recently from 26c87d0 to 63441a4 Compare February 26, 2026 19:24
@troyciesco troyciesco changed the title Updated labs tests to not rely on a specific flags Updated labs tests to not rely on specific flags Feb 26, 2026
}

// NOTE: this test should be rewritten to test the alpha flag independently of the internal ALPHA_FEATURES list
// otherwise we end up in the endless maintenance loop and need to update it every time a feature graduates from alpha
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this test wasn't doing anything. enableDeveloperExperiments doesn't seem to change anything about what labs are returned, i think it just controls whether the tab for editing PRIVATE_FEATURES shows up in the UI.

to further check, i create the inverse of this test ("does not return an alpha flag when dev experiments is false") and it failed.

based on how labs works with getAll() returning all beta and private keys, I don't think a test is needed here.

@troyciesco troyciesco marked this pull request as ready for review February 26, 2026 19:39
@troyciesco troyciesco requested a review from EvanHahn February 26, 2026 20:12
@EvanHahn
Copy link
Contributor

I'll plan to review this next week. Let me know if you want me to look sooner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants