Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Proposal: replace gomonkey with xgo in unit test #161

Open
xhd2015 opened this issue Apr 1, 2024 · 5 comments
Open

Proposal: replace gomonkey with xgo in unit test #161

xhd2015 opened this issue Apr 1, 2024 · 5 comments

Comments

@xhd2015
Copy link

xhd2015 commented Apr 1, 2024

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
Copy link

liminababy commented Apr 1, 2024 via email

@xhd2015
Copy link
Author

xhd2015 commented Apr 2, 2024

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 #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
Copy link
Author

xhd2015 commented Apr 2, 2024

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
Copy link
Author

xhd2015 commented Apr 2, 2024

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
Copy link
Author

xhd2015 commented Apr 2, 2024

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.

@xhd2015 xhd2015 mentioned this issue Apr 2, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@xhd2015 @liminababy