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

Fix test failures that started appearing in CI #526

Merged
merged 3 commits into from Jul 3, 2022

Conversation

jwmcglynn
Copy link
Collaborator

Fix failures likely from CI environment and timing changes

Fix a double-socket-close in ServerConnection, which triggers an STFATAL in UnixSocketHandler, which was showing up on the flaky socket tests. To fix, if destroyPartialConnection is called don't call close, since destroyPartialConnection already closes the socket.

Unset XDG_RUNTIME_DIR before running tests, since it changes the test behavior, and this appears to now be set by the CI system.

Disable ServerFifoPath tests when run as root, since the creation behavior it is testing is disabled as root, and it leads to a REQUIRE failure.

When this double-close occurs, an STFATAL is hit in UnixSocketHandler
since the socket is already closed.

To fix, if `destroyPartialConnection` is called don't call `close`,
since `destroyPartialConnection` already closes the socket.
If the environment sets XDG_RUNTIME_DIR the test behavior changes and
fails a check.

Unset the env var before running the test.
@codecov-commenter
Copy link

Codecov Report

Merging #526 (38489cb) into master (6f89484) will decrease coverage by 0.03%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master     #526      +/-   ##
==========================================
- Coverage   71.34%   71.30%   -0.04%     
==========================================
  Files          49       49              
  Lines        3050     3074      +24     
==========================================
+ Hits         2176     2192      +16     
- Misses        874      882       +8     
Impacted Files Coverage Δ
src/base/ServerConnection.cpp 70.75% <50.00%> (ø)
test/ServerFifoPathTest.cpp 94.24% <73.68%> (-4.16%) ⬇️
src/terminal/TerminalClient.cpp 41.22% <0.00%> (-0.91%) ⬇️
src/base/ClientConnection.cpp 78.18% <0.00%> (+0.90%) ⬆️
src/base/BackedReader.cpp 85.71% <0.00%> (+1.29%) ⬆️
src/base/Connection.cpp 87.82% <0.00%> (+3.47%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f89484...38489cb. Read the comment docs.

@MisterTea MisterTea merged commit eca37f3 into MisterTea:master Jul 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants