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

Unit test failures #447

Closed
weikequ opened this issue Jun 7, 2023 · 2 comments
Closed

Unit test failures #447

weikequ opened this issue Jun 7, 2023 · 2 comments
Assignees
@sam-berning
Labels
bug Something isn't working

Comments

@weikequ
Copy link
Contributor

weikequ commented Jun 7, 2023

Describe the bug
If I follow contributing.md to set up my dev environment, make test-unit fails due to a race condition.

Steps to reproduce
Follow the steps on contributing.md.

Expected behavior
The unit tests should pass

Screenshots or logs

❯ make test-unit
go: downloading github.com/runfinch/common-tests v0.7.0
go: downloading github.com/golang/mock v1.6.0
go: downloading github.com/shirou/gopsutil/v3 v3.23.5
go: downloading github.com/stretchr/testify v1.8.4
go: downloading github.com/davecgh/go-spew v1.1.1
go: downloading github.com/pmezard/go-difflib v1.0.0
go: downloading github.com/tklauser/go-sysconf v0.3.11
go: downloading github.com/shoenig/go-m1cpu v0.1.6
go test github.com/runfinch/finch/benchmark github.com/runfinch/finch/benchmark/all github.com/runfinch/finch/benchmark/container github.com/runfinch/finch/benchmark/vm github.com/runfinch/finch/cmd/finch github.com/runfinch/finch/pkg/command github.com/runfinch/finch/pkg/config github.com/runfinch/finch/pkg/dependency github.com/runfinch/finch/pkg/dependency/vmnet github.com/runfinch/finch/pkg/disk github.com/runfinch/finch/pkg/flog github.com/runfinch/finch/pkg/fmemory github.com/runfinch/finch/pkg/fssh github.com/runfinch/finch/pkg/lima github.com/runfinch/finch/pkg/mocks github.com/runfinch/finch/pkg/path github.com/runfinch/finch/pkg/support github.com/runfinch/finch/pkg/system github.com/runfinch/finch/pkg/version -shuffle on -race
?   	github.com/runfinch/finch/benchmark	[no test files]
ok  	github.com/runfinch/finch/benchmark/all	0.150s [no tests to run]
ok  	github.com/runfinch/finch/benchmark/container	0.357s [no tests to run]
ok  	github.com/runfinch/finch/benchmark/vm	0.252s [no tests to run]
ok  	github.com/runfinch/finch/cmd/finch	0.210s
ok  	github.com/runfinch/finch/pkg/command	0.286s
ok  	github.com/runfinch/finch/pkg/config	0.392s
ok  	github.com/runfinch/finch/pkg/dependency	0.484s
?   	github.com/runfinch/finch/pkg/flog	[no test files]
?   	github.com/runfinch/finch/pkg/fmemory	[no test files]
ok  	github.com/runfinch/finch/pkg/dependency/vmnet	0.200s
?   	github.com/runfinch/finch/pkg/mocks	[no test files]
ok  	github.com/runfinch/finch/pkg/disk	0.160s
ok  	github.com/runfinch/finch/pkg/fssh	0.263s
?   	github.com/runfinch/finch/pkg/system	[no test files]
?   	github.com/runfinch/finch/pkg/version	[no test files]
ok  	github.com/runfinch/finch/pkg/lima	0.374s
ok  	github.com/runfinch/finch/pkg/path	0.301s
-test.shuffle 1686096339395642000
==================
WARNING: DATA RACE
Write at 0x000104832de8 by goroutine 36:
  github.com/lima-vm/lima/pkg/osutil.LimaUser()
      /Users/weikequ/go/pkg/mod/github.com/lima-vm/[email protected]/pkg/osutil/user.go:100 +0x2c
  github.com/runfinch/finch/pkg/support.(*bundleBuilder).copyInFile()
      /Users/weikequ/weikequ/finch/pkg/support/support.go:174 +0x2c0
  github.com/runfinch/finch/pkg/support.(*bundleBuilder).GenerateSupportBundle()
      /Users/weikequ/weikequ/finch/pkg/support/support.go:108 +0x7b0
  github.com/runfinch/finch/pkg/support.TestSupportBundleBuilder_GenerateSupportBundle.func6()
      /Users/weikequ/weikequ/finch/pkg/support/support_test.go:230 +0x740
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1576 +0x188
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1629 +0x40

Previous write at 0x000104832de8 by goroutine 25:
  github.com/lima-vm/lima/pkg/osutil.LimaUser()
      /Users/weikequ/go/pkg/mod/github.com/lima-vm/[email protected]/pkg/osutil/user.go:100 +0x2c
  github.com/runfinch/finch/pkg/support.(*bundleBuilder).copyInFile()
      /Users/weikequ/weikequ/finch/pkg/support/support.go:174 +0x2c0
  github.com/runfinch/finch/pkg/support.(*bundleBuilder).GenerateSupportBundle()
      /Users/weikequ/weikequ/finch/pkg/support/support.go:108 +0x7b0
  github.com/runfinch/finch/pkg/support.TestSupportBundleBuilder_GenerateSupportBundle.func6()
      /Users/weikequ/weikequ/finch/pkg/support/support_test.go:230 +0x740
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1576 +0x188
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1629 +0x40

Goroutine 36 (running) created at:
  testing.(*T).Run()
      /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1629 +0x5e4
  github.com/runfinch/finch/pkg/support.TestSupportBundleBuilder_GenerateSupportBundle()
      /Users/weikequ/weikequ/finch/pkg/support/support_test.go:192 +0x360
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1576 +0x188
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1629 +0x40

Goroutine 25 (running) created at:
  testing.(*T).Run()
      /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1629 +0x5e4
  github.com/runfinch/finch/pkg/support.TestSupportBundleBuilder_GenerateSupportBundle()
      /Users/weikequ/weikequ/finch/pkg/support/support_test.go:192 +0x360
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1576 +0x188
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1629 +0x40
==================
--- FAIL: TestSupport_writePlatformData (0.00s)
--- FAIL: TestSupport_redactSSHKeys (0.00s)
    --- FAIL: TestSupport_redactSSHKeys/should_replace_rsa_key (0.00s)
        testing.go:1446: race detected during execution of test
    --- FAIL: TestSupport_redactSSHKeys/should_replace_ed25519_key (0.00s)
        testing.go:1446: race detected during execution of test
--- FAIL: TestSupport_redactNetworkAddresses (0.00s)
--- FAIL: TestSupport_redactPorts (0.00s)
--- FAIL: TestSupportBundleBuilder_GenerateSupportBundle (0.00s)
    --- FAIL: TestSupportBundleBuilder_GenerateSupportBundle/Generate_support_bundle_with_a_config_file_excluded (0.01s)
        testing.go:1446: race detected during execution of test
    --- FAIL: TestSupportBundleBuilder_GenerateSupportBundle/Generate_support_bundle_with_a_log_file_excluded (0.01s)
        testing.go:1446: race detected during execution of test
    --- FAIL: TestSupportBundleBuilder_GenerateSupportBundle/Generate_support_bundle_with_an_included_file_excluded (0.01s)
        testing.go:1446: race detected during execution of test
    --- FAIL: TestSupportBundleBuilder_GenerateSupportBundle/Generate_support_bundle_with_an_extra_file_included (0.02s)
        testing.go:1446: race detected during execution of test
    --- FAIL: TestSupportBundleBuilder_GenerateSupportBundle/Generate_support_bundle (0.02s)
        testing.go:1446: race detected during execution of test
--- FAIL: TestSupport_bundleFileName (1.00s)
    testing.go:1446: race detected during execution of test
FAIL
FAIL	github.com/runfinch/finch/pkg/support	1.401s
FAIL
make: *** [test-unit] Error 1
@weikequ weikequ added the bug Something isn't working label Jun 7, 2023
@ningziwen ningziwen mentioned this issue Jun 13, 2023
Closed
@sam-berning sam-berning mentioned this issue Jun 13, 2023
Merged
1 task
@sam-berning
Copy link
Contributor

Opened a PR to fix this: #450

sam-berning added a commit that referenced this issue Jun 13, 2023
@sam-berning
test: mocks LimaUser to fix race condition in support bundle unit tes…
9f4b87c 
…ts (#450)

Issue #, if available: #447

*Description of changes:*

There was a race condition in the unit tests for `support-bundle
generate` caused by Lima's `osutil.LimaUser` not being thread-safe. I
don't think there's really a need to make it thread-safe, so I think
it's easier to just wrap and mock it for the unit tests, which I've done
here.

*Testing done:*

```
make test-unit
```


- [x] I've reviewed the guidance in CONTRIBUTING.md


#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

Signed-off-by: Sam Berning <[email protected]>
@ginglis13
Copy link
Contributor

Closing as we've had consistent unit testing since the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sam-berning sam-berning

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ginglis13 @weikequ @sam-berning