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.

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

Problems with setup and teardown Tarantool in tests #147

Open
ligurio opened this issue Mar 13, 2022 · 4 comments
Open

Problems with setup and teardown Tarantool in tests #147

ligurio opened this issue Mar 13, 2022 · 4 comments
Labels
2sp bug Something isn't working teamE

Comments

@ligurio
Copy link
Member

ligurio commented Mar 13, 2022

  1. Tarantool process is not killed when tests fail with panic(). I don't know why. The test has a line defer test_helpers.StopTarantoolWithCleanup(instance), according to blog post Defer, Panic, and Recover deferred function should be executed eve in case of panic():

Panic is a built-in function that stops the ordinary flow of control and begins panicking. When the function F calls panic, execution of F stops, any deferred functions in F are executed normally, and then F returns to its caller. To the caller, F then behaves like a call to panic. The process continues up the stack until all functions in the current goroutine have returned, at which point the program crashes. Panics can be initiated by invoking panic directly. They can also be caused by runtime errors, such as out-of-bounds array accesses.

  1. on next attempt to run tests StartTarantool() is not failed when Tarantool is already run on the same TCP port

Version: d3b5696

@DifferentialOrange DifferentialOrange added bug Something isn't working teamE labels Mar 21, 2022
@ligurio ligurio changed the title Problems with setup and teradown Tarantool in tests Problems with setup and teardown Tarantool in tests Apr 12, 2022
ligurio added a commit that referenced this issue May 24, 2022
Commit "test: handle everything with go test"
(9a6159a) handles everything except a
case when tests are not terminated when Tarantool start is failed due to
busy network port. Patch adds a dumb check that makre desired port is
not busy.

Part of #147
ligurio added a commit that referenced this issue May 24, 2022
Commit "test: handle everything with go test"
(9a6159a) handles everything except a
case when tests are not terminated when Tarantool start is failed due to
busy network port. Patch adds a dumb check that make sure desired port
is not busy.

Part of #147
ligurio added a commit that referenced this issue Jun 16, 2022
Commit "test: handle everything with go test"
(9a6159a) handles everything except a
case when tests are not terminated when Tarantool start is failed due to
busy network port. Patch adds a dumb check that make sure desired port
is not busy.

Part of #147
@oleg-jukovec
Copy link
Collaborator

It may be helpful:
golang/go#37206

@oleg-jukovec
Copy link
Collaborator

oleg-jukovec commented Jul 1, 2022

I have found only one simple way: kill child processes .

I works but this is OS-dependent code. It should be implemented after supporting of all target platforms on CI, after #157 at least.

an example for Linux
diff --git a/test_helpers/cmd.go b/test_helpers/cmd.go
new file mode 100644
index 0000000..94b62ff
--- /dev/null
+++ b/test_helpers/cmd.go
@@ -0,0 +1,14 @@
+//go:build !linux
+
+package test_helpers
+
+import (
+       "os/exec"
+)
+
+// CommandKillOnExit tries to create a commant that will killed after a parent,
+// see a problem: https://github.com/golang/go/issues/37206
+// The default implementation just call a exec.Comman.
+func CommandKillOnExit(name string, arg ...string) *exec.Cmd {
+       return exec.Command(name, arg...)
+}
diff --git a/test_helpers/cmd_linux.go b/test_helpers/cmd_linux.go
new file mode 100644
index 0000000..5721376
--- /dev/null
+++ b/test_helpers/cmd_linux.go
@@ -0,0 +1,18 @@
+//go:build linux
+
+package test_helpers
+
+import (
+       "os/exec"
+       "syscall"
+)
+
+// CommandKillOnExit tries to create a commant that will killed after a parent,
+// see a problem: https://github.com/golang/go/issues/37206
+func CommandKillOnExit(name string, arg ...string) *exec.Cmd {
+       cmd := exec.Command(name, arg...)
+       cmd.SysProcAttr = &syscall.SysProcAttr{
+               Pdeathsig: syscall.SIGTERM,
+       }
+       return cmd
+}
diff --git a/test_helpers/main.go b/test_helpers/main.go
index 5c9d513..1e4a6a5 100644
--- a/test_helpers/main.go
+++ b/test_helpers/main.go
@@ -184,7 +184,7 @@ func RestartTarantool(inst *TarantoolInstance) error {
 func StartTarantool(startOpts StartOpts) (TarantoolInstance, error) {
        // Prepare tarantool command.
        var inst TarantoolInstance
-       inst.Cmd = exec.Command("tarantool", startOpts.InitScript)
+       inst.Cmd = CommandKillOnExit("tarantool", startOpts.InitScript)
 
        inst.Cmd.Env = append(
                os.Environ(),

Another way is to add guard code to each test case which will catch a panic.

func catchPanic() {
    if (recover() != nil) {
        // stop Tarantool
    }
}

func TestCase(t *testing.T) {
    defer catchPanic()
    // test code
}

But there are may be issues with subtests.

@filonenko-mikhail
Copy link

other solution is to use unix sockets

@oleg-jukovec
Copy link
Collaborator

other solution is to use unix sockets

Could you please explain how this can help?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2sp bug Something isn't working teamE
Projects
None yet
Development

No branches or pull requests

6 participants