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

tests: investigate testdriver race #1104

Closed
leitzler opened this issue Sep 11, 2021 · 0 comments · Fixed by #1105
Closed

tests: investigate testdriver race #1104

leitzler opened this issue Sep 11, 2021 · 0 comments · Fixed by #1105

Comments

@leitzler
Copy link
Member

Two recent failed test:

Example:

WARNING: DATA RACE
Read at 0x000000934e50 by goroutine 23:
  github.com/govim/govim/testdriver.NewTestDriver()
      /home/runner/govim/testdriver/testdriver.go:220 +0x147d
  github.com/govim/govim_test.TestScripts.func1.1()
      /home/runner/govim/govim_test.go:123 +0xc24
  github.com/rogpeppe/go-internal/testscript.(*TestScript).setup()
      /home/runner/gopath/pkg/mod/github.com/rogpeppe/go-internal@v1.8.0/testscript/testscript.go:347 +0x144c
  github.com/rogpeppe/go-internal/testscript.(*TestScript).run()
      /home/runner/gopath/pkg/mod/github.com/rogpeppe/go-internal@v1.8.0/testscript/testscript.go:402 +0x12c
  github.com/rogpeppe/go-internal/testscript.RunT.func1()
      /home/runner/gopath/pkg/mod/github.com/rogpeppe/go-internal@v1.8.0/testscript/testscript.go:247 +0x499
  github.com/rogpeppe/go-internal/testscript.tshim.Run.func1()
      /home/runner/gopath/pkg/mod/github.com/rogpeppe/go-internal@v1.8.0/testscript/testscript.go:183 +0x52
  testing.tRunner()
      /go/src/testing/testing.go:1123 +0x202

Previous write at 0x000000934e50 by goroutine 12:
  github.com/govim/govim/testdriver.NewTestDriver()
      /home/runner/govim/testdriver/testdriver.go:227 +0x14f8
  github.com/govim/govim_test.TestScripts.func1.1()
      /home/runner/govim/govim_test.go:123 +0xc24
  github.com/rogpeppe/go-internal/testscript.(*TestScript).setup()
      /home/runner/gopath/pkg/mod/github.com/rogpeppe/go-internal@v1.8.0/testscript/testscript.go:347 +0x144c
  github.com/rogpeppe/go-internal/testscript.(*TestScript).run()
      /home/runner/gopath/pkg/mod/github.com/rogpeppe/go-internal@v1.8.0/testscript/testscript.go:402 +0x12c
  github.com/rogpeppe/go-internal/testscript.RunT.func1()
      /home/runner/gopath/pkg/mod/github.com/rogpeppe/go-internal@v1.8.0/testscript/testscript.go:247 +0x499
  github.com/rogpeppe/go-internal/testscript.tshim.Run.func1()
      /home/runner/gopath/pkg/mod/github.com/rogpeppe/go-internal@v1.8.0/testscript/testscript.go:183 +0x52
  testing.tRunner()
      /go/src/testing/testing.go:1123 +0x202

Goroutine 23 (running) created at:
  testing.(*T).Run()
      /go/src/testing/testing.go:1168 +0x5bb
  github.com/rogpeppe/go-internal/testscript.tshim.Run()
      /home/runner/gopath/pkg/mod/github.com/rogpeppe/go-internal@v1.8.0/testscript/testscript.go:182 +0xb0
  github.com/rogpeppe/go-internal/testscript.RunT()
      /home/runner/gopath/pkg/mod/github.com/rogpeppe/go-internal@v1.8.0/testscript/testscript.go:223 +0x634
  github.com/rogpeppe/go-internal/testscript.Run()
      /home/runner/gopath/pkg/mod/github.com/rogpeppe/go-internal@v1.8.0/testscript/testscript.go:160 +0x384
  github.com/govim/govim_test.TestScripts.func1()
      /home/runner/govim/govim_test.go:50 +0x2d5
  testing.tRunner()
      /go/src/testing/testing.go:1123 +0x202

Goroutine 12 (running) created at:
  testing.(*T).Run()
      /go/src/testing/testing.go:1168 +0x5bb
  github.com/rogpeppe/go-internal/testscript.tshim.Run()
      /home/runner/gopath/pkg/mod/github.com/rogpeppe/go-internal@v1.8.0/testscript/testscript.go:182 +0xb0
  github.com/rogpeppe/go-internal/testscript.RunT()
      /home/runner/gopath/pkg/mod/github.com/rogpeppe/go-internal@v1.8.0/testscript/testscript.go:223 +0x634
  github.com/rogpeppe/go-internal/testscript.Run()
      /home/runner/gopath/pkg/mod/github.com/rogpeppe/go-internal@v1.8.0/testscript/testscript.go:160 +0x384
  github.com/govim/govim_test.TestScripts.func1()
      /home/runner/govim/govim_test.go:50 +0x2d5
  testing.tRunner()
      /go/src/testing/testing.go:1123 +0x202
==================
--- FAIL: TestScripts (0.00s)
    --- FAIL: TestScripts/scripts (0.00s)
        --- FAIL: TestScripts/scripts/bad_function (0.46s)
            testscript.go:397: 
                WORK=/artefacts/govim-race/script-bad_function
                # Test the handling of functions that return errors
                # call a bad function (0.094s)
                PASS
                
            testing.go:1038: race detected during execution of test
        --- FAIL: TestScripts/scripts/unscheduled (0.92s)
            testscript.go:397: 
                WORK=/artefacts/govim-race/script-unscheduled
                # Test that unscheduled callbacks can schedule work (0.557s)
                PASS
                
            testing.go:1038: race detected during execution of test
FAIL
leitzler added a commit that referenced this issue Sep 11, 2021
@leitzler
testdriver: fix data race introduced in #1101
67ec479 
When #1101 was merged I accidentally introduced a data race by modifying
the global VimCommand. We now make a local copy that is modified
instead.

Closes #1104
@leitzler leitzler mentioned this issue Sep 11, 2021
Merged
leitzler added a commit that referenced this issue Sep 12, 2021
@leitzler
testdriver: fix data race introduced in #1101
eeaa394 
With #1101 a data race was accidentally introduced where the global
testsetup.VimCommand was overwritten.
We now make a local copy that is modified instead.

Fixing this revealed that #1101 wasn't enough. Testscript conditions
that matches a vim version also used the testscript subcommand instead
of real vim. It now uses the real vim.

Closes #1104
leitzler added a commit that referenced this issue Sep 13, 2021
@leitzler
testdriver: fix data race introduced in #1101
9265905 
With #1101 a data race was accidentally introduced where the global
testsetup.VimCommand was overwritten.
We now make a local copy that is modified instead.

Fixing this revealed that #1101 wasn't enough. Testscript conditions
that matches a vim version also used the testscript subcommand instead
of real vim. It now uses the real vim.

Closes #1104
leitzler added a commit that referenced this issue Sep 14, 2021
@leitzler
testdriver: fix data race introduced in #1101
5a0756f 
With #1101 a data race was accidentally introduced where the global
testsetup.VimCommand was overwritten.
We now make a local copy that is modified instead.

Fixing this revealed that #1101 wasn't enough. Testscript conditions
that matches a vim version also used the testscript subcommand instead
of real vim. It now uses the real vim.

Closes #1104
leitzler added a commit that referenced this issue Sep 15, 2021
@leitzler
testdriver: fix data race introduced in #1101
f962214 
With #1101 a data race was accidentally introduced where the global
testsetup.VimCommand was overwritten.
We now make a local copy that is modified instead.

Fixing this revealed that #1101 wasn't enough. Testscript conditions
that matches a vim version also used the testscript subcommand instead
of real vim. It now uses the real vim.

Closes #1104
leitzler added a commit that referenced this issue Sep 19, 2021
@leitzler
testdriver: fix data race introduced in #1101
3b0a833 
With #1101 a data race was accidentally introduced where the global
testsetup.VimCommand was overwritten.
We now make a local copy that is modified instead.

Fixing this revealed that #1101 wasn't enough. Testscript conditions
that matches a vim version also used the testscript subcommand instead
of real vim. It now uses the real vim.

Closes #1104
leitzler added a commit that referenced this issue Sep 20, 2021
@leitzler
testdriver: fix data race introduced in #1101 (#1105)
385dbe5 
With #1101 a data race was accidentally introduced where the global
testsetup.VimCommand was overwritten.
We now make a local copy that is modified instead.

Fixing this revealed that #1101 wasn't enough. Testscript conditions
that matches a vim version also used the testscript subcommand instead
of real vim. It now uses the real vim.

Closes #1104
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

Successfully merging a pull request may close this issue.

testdriver: fix data race introduced in #1101 govim/govim
1 participant
@leitzler