ci: build and test tntcxx on macos 11 and 12 #55
Conversation
drewdzzz
force-pushed
the
macos-ci
branch
3 times, most recently
from
843390f to
1cd2be4
Compare
drewdzzz
force-pushed
the
macos-ci
branch
16 times, most recently
from
62c3658 to
9a4859e
Compare
| * will kill its child process, which is Tarantool, and exit. | ||
| * In the case, when the intermediate process fails to fork Tarantool, it kills | ||
| * the parent process and terminates. | ||
| */ |
There was a problem hiding this comment.
I agree with @alyapunov in the sense that this is code is really sophistacted, but I guess there is no other way to implement this approach on macOS, so I see two ways to make life easier for us:
- Since the approach @drewdzzz proposes here is portable, let's use it everywhere: i.e., drop the Linux version and only leave this one.
- Abandon this approach entirely, and instead refactor our test code to save the Tarantool PID (perhaps, create some
TarantoolInstanceobject), and clean it up properly when testing fails. An approach like this is used in go-tarantool, for instance 1. Not sure what to do, if the test crashes though (seems like the process leaks in this case), I guess we should consult with someone from @tarantool/ecosystem, like @DifferentialOrange or @0x501D.
Footnotes
There was a problem hiding this comment.
Not sure what to do, if the test crashes though
go-tarantool is also unable to shutdown Tarantool processes in case of a crash now, so this problem is unresolved for us as well.
There was a problem hiding this comment.
Not sure what to do, if the test crashes though
go-tarantool is also unable to shutdown Tarantool processes in case of a crash now, so this problem is unresolved for us as well.
This is an implementation flaw. It could be implemented in Go for panic in fact:
tarantool/go-tarantool#147 (comment)
But there is no solution to the problem in go-tarantool that would be suitable for C++.
There was a problem hiding this comment.
Why do you think that a process leak happens?
I crashed the test and all processes have been deleted.
There was a problem hiding this comment.
Why do you think that a process leak happens?
I mean if we go for option 2 that I listed.
There was a problem hiding this comment.
In this case, kill is signal-safe, so we could kill Tarantool from signal handler.
There was a problem hiding this comment.
Yeah, I also thought about this, we could add signal handlers from recoverable signals (i.e., even go can't do anything about SIGKILL).
Another path we can take is setting up and cleaning up Tarantool externally (i.e., via the test harness or some shell scripting), but it may not be scalable or convenient.
There was a problem hiding this comment.
I guess it's time for @alyapunov to take the decision how we should proceed here.
drewdzzz
force-pushed
the
macos-ci
branch
4 times, most recently
from
15f8961 to
66eea01
Compare
drewdzzz
force-pushed
the
macos-ci
branch
4 times, most recently
from
f62deb7 to
a25175c
Compare
| make -j | ||
|
|
||
| - name: test without SSL | ||
| if: matrix.test-ssl == 'Disabled' | ||
| run: cd build && ctest --output-on-failure -E ClientSSL.test |
There was a problem hiding this comment.
[feel free to ignore, just for your knowledge]
Nit: you could use the ${{ matrix.test-ssl == 'Disabled' && 'ClientSSL.test' || ''}} ternary operator syntax (see example here) just like in Lua to simplify the code and have less branches.
The test uses std::vector, but it was not included by mistake.
The file uses std::size, std::begin and other such methods, but header with definitions was not included by mistake.
Function launchTarantool creates a process, which is killed when the parent process is dead. Since it uses prctl, it works only on Linux - on other platforms Tarantool is not killed and ctest hangs. Let's add POSIX-compatible implementation of launchTarantool for non-Linux platforms.
The project is tested in both Release and Debug modes on each version of macos. Tarantool is installed from brew, the last stable version is used. The workflow is defined in the same file as Ubuntu workflow, so the file is renamed to "testing".
We are unable to test SSL conection without tarantool-ee, but we still can build tarantool with SSL and skip ClientSSL test. Let's do so.
The project is tested in both Release and Debug modes on each version of
macos. Tarantool is installed from brew, the last stable version is used.