fix(export): add id of rollouts/constraints to export
In https://github.com/orgs/flipt-io/discussions/4311, the audit logs were already improved. Now this is a small follow up. I want to add the id's of the rollouts/segments to the flipt export. This way, when an audit log comes in, the previous version of that record can easily be looked up using it's id in a previous export.
This PR simply adds the ids to the export interfaces.
The tests still need to be updated however. That is of course, if you agree with the changes.
Also this is my first ever open source PR :)
congrats on the your first open source contribution @crossy-l !! thank you!
Thanks. I will look into the test cases and bump the version in the following days.
@markphelps all right, now trying to manually update all test cases seems incredibly tedious. I put a little snippet inside the exporter_test to generate new fixtures like so (this is only a small demonstration, not that I actually want that inside the exporter_test):
t.Run(fmt.Sprintf("%s (%s)", tc.name, ext), func(t *testing.T) {
var (
exporter = NewExporter(tc.lister, tc.namespaces, tc.allNamespaces, tc.sortByKey)
b = new(bytes.Buffer)
)
err := exporter.Export(context.Background(), ext, b)
require.NoError(t, err)
// - snippet here
// Overwrite existing fixture file
outputPath := tc.path + "." + string(ext)
err = os.WriteFile(outputPath, b.Bytes(), 0644)
require.NoError(t, err)
b = bytes.NewBuffer(b.Bytes())
// - end of snippet
in, err := os.ReadFile(tc.path + "." + string(ext))
require.NoError(t, err)
var (
expected = ext.NewDecoder(bytes.NewReader(in))
found = ext.NewDecoder(b)
)
// handle newline delimited JSON
for {
var exp, fnd any
eerr := expected.Decode(&exp)
ferr := found.Decode(&fnd)
require.Equal(t, eerr, ferr)
if errors.Is(ferr, io.EOF) {
break
}
require.NoError(t, ferr)
assert.Equal(t, exp, fnd)
}
})
This however introduces lot's of small formatting differences between the existing fixtures. I am wondering if you would also want some kind of command for generating fixtures based on the expected test. Of course these generated fixtures would need to be manually checked. However when changing something in the export functionality it will be trivial to update tests.
I will just commit the generated fixtures so you can see the differences. (And sorry for the force push on the branch, I committed with the wrong config.)
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 63.87%. Comparing base (563759a) to head (12a61da).
:warning: Report is 11 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #4359 +/- ##
=======================================
Coverage 63.87% 63.87%
=======================================
Files 172 172
Lines 17659 17661 +2
=======================================
+ Hits 11279 11281 +2
Misses 5705 5705
Partials 675 675
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 63.87% <100.00%> (+<0.01%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
@crossy-l dont worry about the formatting I'll run our formatter prettier on those files
Hello, is this done on my part? The integration tests seem to pass at least when I tried it locally. Can this be merged? @markphelps
sorry for the delay @crossy-l !! thank you for this contribution!
This PR currently has a merge conflict. Please resolve this and then re-add the automerge label.