Proposal: replace gomonkey with xgo in unit test

xhd2015 commented 3 months ago

Hello guys, I'm here planning to rewrite some unit tests in the project, switching from gomonkey to xgo.

Xgo is a more go-native mocking library and provides simpler API and smoother user experience compared to gomonkey.

I'm the creator of xgo, the project can be found at https://github.com/xhd2015/xgo.

I'll create some PRs to demonstrate that xgo has better surface and lower brain overhead, wish me lucky!

liminababy commented 3 months ago

我已经收到你的邮件~(≧▽≦)/~啦啦啦！谢谢~

xhd2015 commented 3 months ago

Update: I have replaced gomonkey.ApplyFunc with xgo's mock.Patch in the chaosmeta-inject-operator submodule.

All mock works, except the build failure as described in https://github.com/traas-stack/chaosmeta/issues/162

cd chaosmeta-inject-operator 
xgo test -v -run '^(Test_initProcess|TestClusterPendingPodExecutor_Inject|TestInjectPhaseHandler_SolveCreated_OneToRunning|TestInjectPhaseHandler_SolveCreated_OneInjectFailedInTwo|TestInjectPhaseHandler_SolveRunning_TwoQueryFailedInThree|TestInjectPhaseHandler_SolveRunning_TwoFailed|TestRecoverPhaseHandler_SolveCreated_OneToRunning|TestRecoverPhaseHandler_SolveRunning)$' ./...

Output:

=== RUN   TestConvertDuration
=== RUN   TestConvertDuration/value_error
=== RUN   TestConvertDuration/unit_error
=== RUN   TestConvertDuration/s,_true
=== RUN   TestConvertDuration/m,_true
=== RUN   TestConvertDuration/h,_true
--- PASS: TestConvertDuration (0.00s)
    --- PASS: TestConvertDuration/value_error (0.00s)
    --- PASS: TestConvertDuration/unit_error (0.00s)
    --- PASS: TestConvertDuration/s,_true (0.00s)
    --- PASS: TestConvertDuration/m,_true (0.00s)
    --- PASS: TestConvertDuration/h,_true (0.00s)
=== RUN   TestAPIs
Running Suite: Webhook Suite - /Users/xhd2015/Projects/xhd2015/chaosmeta/chaosmeta-inject-operator/api/v1alpha1
===============================================================================================================
Random Seed: 1712028449

Will run 0 of 0 specs

Ran 0 of 0 Specs in 0.000 seconds
SUCCESS! -- 0 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestAPIs (0.00s)
PASS
ok      github.com/traas-stack/chaosmeta/chaosmeta-inject-operator/api/v1alpha1 4.564s
=== RUN   Test_solveRange
=== RUN   Test_solveRange/all_success
=== RUN   Test_solveRange/count_success
=== RUN   Test_solveRange/count_more_then_initial_length
=== RUN   Test_solveRange/percent_success
--- PASS: Test_solveRange (0.00s)
    --- PASS: Test_solveRange/all_success (0.00s)
    --- PASS: Test_solveRange/count_success (0.00s)
    --- PASS: Test_solveRange/count_more_then_initial_length (0.00s)
    --- PASS: Test_solveRange/percent_success (0.00s)
=== RUN   Test_initProcess
--- PASS: Test_initProcess (0.00s)
=== RUN   Test_solveFinalizer
--- PASS: Test_solveFinalizer (0.00s)
=== RUN   TestAPIs
Running Suite: Controller Suite - /Users/xhd2015/Projects/xhd2015/chaosmeta/chaosmeta-inject-operator/controllers
=================================================================================================================
Random Seed: 1712028453

Will run 0 of 0 specs

Ran 0 of 0 Specs in 0.000 seconds
SUCCESS! -- 0 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestAPIs (0.00s)
PASS
ok      github.com/traas-stack/chaosmeta/chaosmeta-inject-operator/controllers  9.122s
=== RUN   TestGoroutinePool
--- PASS: TestGoroutinePool (2.00s)
=== RUN   TestClusterCtrl
--- PASS: TestClusterCtrl (2.00s)
=== RUN   TestIsTimeout
=== RUN   TestIsTimeout/no_auto_recover
=== RUN   TestIsTimeout/no_auto_recover#01
=== RUN   TestIsTimeout/time_format_error
=== RUN   TestIsTimeout/timeout
=== RUN   TestIsTimeout/timeout#01
--- PASS: TestIsTimeout (0.00s)
    --- PASS: TestIsTimeout/no_auto_recover (0.00s)
    --- PASS: TestIsTimeout/no_auto_recover#01 (0.00s)
    --- PASS: TestIsTimeout/time_format_error (0.00s)
    --- PASS: TestIsTimeout/timeout (0.00s)
    --- PASS: TestIsTimeout/timeout#01 (0.00s)
=== RUN   TestGetArgs
=== RUN   TestGetArgs/normal
=== RUN   TestGetArgs/more_than
=== RUN   TestGetArgs/oneEmpty
--- PASS: TestGetArgs (0.00s)
    --- PASS: TestGetArgs/normal (0.00s)
    --- PASS: TestGetArgs/more_than (0.00s)
    --- PASS: TestGetArgs/oneEmpty (0.00s)
PASS
ok      github.com/traas-stack/chaosmeta/chaosmeta-inject-operator/pkg/common   10.737s
=== RUN   TestClusterPendingPodExecutor_Inject
--- PASS: TestClusterPendingPodExecutor_Inject (14.00s)
=== RUN   Test_getPatchLabels
=== RUN   Test_getPatchLabels/normal
=== RUN   Test_getPatchLabels/value_empty
=== RUN   Test_getPatchLabels/empty_map
=== RUN   Test_getPatchLabels/empty
=== RUN   Test_getPatchLabels/null
--- PASS: Test_getPatchLabels (0.00s)
    --- PASS: Test_getPatchLabels/normal (0.00s)
    --- PASS: Test_getPatchLabels/value_empty (0.00s)
    --- PASS: Test_getPatchLabels/empty_map (0.00s)
    --- PASS: Test_getPatchLabels/empty (0.00s)
    --- PASS: Test_getPatchLabels/null (0.00s)
=== RUN   Test_getBackupLabels
=== RUN   Test_getBackupLabels/normal
=== RUN   Test_getBackupLabels/normal#01
--- PASS: Test_getBackupLabels (0.00s)
    --- PASS: Test_getBackupLabels/normal (0.00s)
    --- PASS: Test_getBackupLabels/normal#01 (0.00s)
PASS
ok      github.com/traas-stack/chaosmeta/chaosmeta-inject-operator/pkg/executor/cloudnativeexecutor     24.822s
=== RUN   TestConvertStatus
=== RUN   TestConvertStatus/inject_created
=== RUN   TestConvertStatus/inject_success
=== RUN   TestConvertStatus/inject_error
=== RUN   TestConvertStatus/inject_destroyed
=== RUN   TestConvertStatus/recover_created
=== RUN   TestConvertStatus/recover_success
=== RUN   TestConvertStatus/recover_error
=== RUN   TestConvertStatus/recover_destroyed
--- PASS: TestConvertStatus (0.00s)
    --- PASS: TestConvertStatus/inject_created (0.00s)
    --- PASS: TestConvertStatus/inject_success (0.00s)
    --- PASS: TestConvertStatus/inject_error (0.00s)
    --- PASS: TestConvertStatus/inject_destroyed (0.00s)
    --- PASS: TestConvertStatus/recover_created (0.00s)
    --- PASS: TestConvertStatus/recover_success (0.00s)
    --- PASS: TestConvertStatus/recover_error (0.00s)
    --- PASS: TestConvertStatus/recover_destroyed (0.00s)
PASS
ok      github.com/traas-stack/chaosmeta/chaosmeta-inject-operator/pkg/executor/remoteexecutor/base     13.093s
=== RUN   TestParseContainerID
=== RUN   TestParseContainerID/success
=== RUN   TestParseContainerID/success_default
=== RUN   TestParseContainerID/fault_empty
=== RUN   TestParseContainerID/fault_err_format
--- PASS: TestParseContainerID (0.00s)
    --- PASS: TestParseContainerID/success (0.00s)
    --- PASS: TestParseContainerID/success_default (0.00s)
    --- PASS: TestParseContainerID/fault_empty (0.00s)
    --- PASS: TestParseContainerID/fault_err_format (0.00s)
=== RUN   TestParseNodeInfo
=== RUN   TestParseNodeInfo/success
=== RUN   TestParseNodeInfo/success#01
--- PASS: TestParseNodeInfo (0.00s)
    --- PASS: TestParseNodeInfo/success (0.00s)
    --- PASS: TestParseNodeInfo/success#01 (0.00s)
PASS
ok      github.com/traas-stack/chaosmeta/chaosmeta-inject-operator/pkg/model    11.623s
=== RUN   TestInjectPhaseHandler_SolveCreated_OneToRunning
--- PASS: TestInjectPhaseHandler_SolveCreated_OneToRunning (0.00s)
=== RUN   TestInjectPhaseHandler_SolveCreated_OneInjectFailedInTwo
--- PASS: TestInjectPhaseHandler_SolveCreated_OneInjectFailedInTwo (0.00s)
=== RUN   TestInjectPhaseHandler_SolveRunning_TwoQueryFailedInThree
--- PASS: TestInjectPhaseHandler_SolveRunning_TwoQueryFailedInThree (1.00s)
=== RUN   TestInjectPhaseHandler_SolveRunning_TwoFailed
--- PASS: TestInjectPhaseHandler_SolveRunning_TwoFailed (1.00s)
=== RUN   Test_solveFinalStatus
--- PASS: Test_solveFinalStatus (0.00s)
PASS
ok      github.com/traas-stack/chaosmeta/chaosmeta-inject-operator/pkg/phasehandler/inject      15.880s
=== RUN   TestRecoverPhaseHandler_SolveCreated_OneToRunning
--- PASS: TestRecoverPhaseHandler_SolveCreated_OneToRunning (0.00s)
=== RUN   TestRecoverPhaseHandler_SolveRunning
--- PASS: TestRecoverPhaseHandler_SolveRunning (0.00s)
PASS
ok      github.com/traas-stack/chaosmeta/chaosmeta-inject-operator/pkg/phasehandler/recover     15.773s

xhd2015 commented 3 months ago

Compare with gomonkey, which outputs error with some cases:

--- FAIL: Test_initProcess (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x58 pc=0x1fc5b82]

xhd2015 commented 3 months ago

Conclusion: xgo can replace gomonkey totally, provides the following advantages:

Compatibility: xgo provides support through go1.17 to go1.22, for all OS and Arch, while gomonkey has limited support
Stability: xgo does not need to modify readonly area, so on MacOS developers does not need any extra setup
Concurrent safety: xgo has built-in concurrent safety, patchings are isolated among goroutines, tests do not affect each other. While gomonkey is not concurrently safe.

xhd2015 commented 3 months ago

With all that said, I will soon issue a PR to replace gomonkey in the submodule chaosmeta-inject-operator.

If that seems ok to the repository author, I will replace them all in all submodules.

traas-stack / chaosmeta

Proposal: replace gomonkey with xgo in unit test #161