-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
testutils: Add script-based test utilities #54
Conversation
|
6da0a1d
to
ac2acec
Compare
ac2acec
to
98d33c3
Compare
98d33c3
to
03fcce0
Compare
types.go
Outdated
@@ -226,6 +230,8 @@ type TableMeta interface { | |||
secondary() map[string]anyIndexer // Secondary indexers (if any) | |||
sortableMutex() internal.SortableMutex // The sortable mutex for locking the table for writing | |||
anyChanges(txn WriteTxn) (anyChangeIterator, error) | |||
proto() any | |||
unmarshalYAML(data string) (any, error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we add this here, part of the TableMeta as it's only usage will be part of this script.go
, or extract to a different package/even the scriptutils.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'll need to revisit these weird ones a bit and see how I can mostly hide them. They do need to be in the genTable
though as that's the place where we know what Obj
is, so I cannot put it anywhere else. I could of course put these behind something else than TableMeta
interface and cast things around in any_table.go
etc. Not sure what the benefit is.
The yaml thing was a bit annoying as I need to call yaml.Unmarshal
with a concrete thing and if it goes via the any
(e.g. by using proto()
to get the zero object) it'll just end up producing map[string]any
as a result. Will have to play around with it and see if I can get rid of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I don't think there's a way around having unmarshalYAML
here. We need to call yaml.Unmarshal
on the concrete type as Unmarshal
reflects on the type of the destination value and if we go via any
we lose the type information at runtime. What I'll do though is split of the weird internal things into tableInternal
interface so it's not so visible.
d8dfe31
to
5fd4882
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Add generic testscript commands for testing against StateDB tables. This allows implementing tests as scripts, which becomes useful when tests perform multiple steps on tables and need to verify the output each step. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
5fd4882
to
346f14f
Compare
Add testutils with functions for creating commands for testscript.
This allows implementing tests as scripts, which becomes useful when tests perform multiple steps and need to verify one or more tables on each step. This aims to be a better way of writing test-suites that work against a set of input files and golden output files.
Each test suite would define their own set of commands with some table-based ones built using the utils and perhaps some specific to the thing being tested.
testscript docs: https://pkg.go.dev/github.com/rogpeppe/go-internal/testscript
blog posts for better intro: https://encore.dev/blog/testscript-hidden-testing-gem, https://atlasgo.io/blog/2024/09/09/how-go-tests-go-test
and the reason I didn't use rsc.io/script: rsc/script#4