search

numaproj / numaplane

Control Plane for Numaproj
Apache License 2.0
4 stars 3 forks source link

PipelineRollout Tests #41

Closed afugazzotto closed 2 weeks ago

afugazzotto commented 2 weeks ago

Fixes #20

Modifications

Updated the suite_test.go file to be able to setup an environment to test PipelineRollout reconciliation logic using Numaflow CRDs. Also, added test cases to pipelinerollout_controller_test.go file to test create, update, and delete of a PipelineRollout resource.

Verification

Issued make test command and verify all tests passed.

juliev0 commented 2 weeks ago

This is very elegant....but I'm not sure if I see any cases of things going wrong? should that be added to a subsequent PR? (at least the case of invalid yaml, not sure what else off the top of my head)

juliev0 commented 2 weeks ago

It should be noted this unit test doesn't really cover things on an individual function level, but maybe this is really the only good way to test Reconcile(). But it makes me wonder if we want to include individual function tests for some functions that are strictly related to data processing now or in the future. Any functions that are trivial it's probably not worth doing, but I'm not sure if any of our functions are non-trivial.

xdevxy commented 2 weeks ago

@juliev0 do you want to take another look? Otherwise I will merge this. Thanks!

juliev0 commented 2 weeks ago

@juliev0 do you want to take another look? Otherwise I will merge this. Thanks!

please do, thanks!