feat: "cdk flags" command reports active and missing feature flags (unstable) #699
Conversation
vivian12300
requested a deployment
to
integ-approval
GitHub Actions
Waiting
vivian12300
requested a deployment
to
integ-approval
GitHub Actions
Waiting
vivian12300
requested a deployment
to
integ-approval
GitHub Actions
Waiting
rix0rrr
changed the title
…o read-report-api
| expect(plainTextOutput).toContain('Feature Flags Report:'); | ||
| expect(plainTextOutput).toContain('Feature Flag Name'); | ||
| expect(plainTextOutput).toContain('Recommended Value'); | ||
| expect(plainTextOutput).toContain('User Value'); |
There was a problem hiding this comment.
You are asserting a lot of things here. Why are you asserting this, and what are you protecting against?
There was a problem hiding this comment.
I want to make sure the output being displayed to the user matches what I'm expecting (the headers and the information about the flag).
There was a problem hiding this comment.
Yes, but why? Or put another way: what are you protecting that someone might accidentally break? Because a lot of this is now double bookkeeping, but not in a way that seems immediately valueable. Also, testing that the text you unconditionally print did actually get unconditionally printed... is that worth it?
If you just want to eyeball the output, expect(...).toMatchSnapshot() or expect(...).toMatchInlineSnapshot() might be appropriate.
If someone wants to change a column header, they now need to change that in two places (change it in the code, and change it here). That may be worth it if that column name encodes a public contract, but it's just annoying busywork if it doesn't.
My point is that tests aren't free: they are additional code to write, compile, run, and maintain. So I'd hope they assert something worthwhile, to pull their weight
(And this is a very trivial case, but that generalizes to more complicated tests)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #699 +/- ##
==========================================
- Coverage 79.38% 79.34% -0.04%
==========================================
Files 49 50 +1
Lines 7209 7278 +69
Branches 809 821 +12
==========================================
+ Hits 5723 5775 +52
- Misses 1467 1484 +17
Partials 19 19
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cdk flags, which displays user's current feature flag configuration in the form of a tableBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license