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

Skip some tests on non-MPTCP kernels, part 2. #216

Merged
merged 6 commits into from
Feb 3, 2022

Conversation

matttbe
Copy link
Member

@matttbe matttbe commented Feb 3, 2022

I had to add two commits on top of #215.

I'm adding them in this new PR as I cannot edit @ossama-othman 's one, I hope that's OK :)

In short, start-stop was not working with a non-MPTCP kernel and a 'Non-interactive sudo capable'.

While at it, I also skip the test instead of doing nothing is we don't have a 'Non-interactive sudo capable'.

Now all tests are OK on my test VM:

PASS: test-plugin
PASS: test-network-monitor
SKIP: test-path-manager
SKIP: test-commands
PASS: test-configuration
PASS: test-id-manager
PASS: test-sockaddr
PASS: test-addr-info
PASS: test-cxx-build
PASS: test-bad-log-empty
PASS: test-bad-log-long
PASS: test-bad-log-short
PASS: test-bad-option
PASS: test-bad-path-manager
PASS: test-bad-plugin-dir
SKIP: test-start-stop
SKIP: test-mptcpwrap
============================================================================
Testsuite summary for mptcpd 0.9
============================================================================
# TOTAL: 17
# PASS:  13
# SKIP:  4
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================

Ossama Othman and others added 6 commits February 2, 2022 13:23
The mptcpd unit tests test-commands, test-path-manager and
test-mptcpwrap require a MPTCP capable kernel to run.  Make those
tests return with an exit status (77) that causes the Automake test
driver script, i.e. `test-driver' to treat the tests as skipped.  This
allows the test suite to succeed when MPTCP is not available in the
kernel.

The test summary from a `make check' run would look similar to the
following when run on a non-MPTCP capable kernel:

 ...
 PASS: test-network-monitor
 SKIP: test-path-manager
 SKIP: test-commands
 PASS: test-configuration
 ...
 SKIP: test-mptcpwrap
 =========================================================================
 Testsuite summary for mptcpd 0.9
 =========================================================================
 # TOTAL: 17
 # PASS:  14
 # SKIP:  3
 # XFAIL: 0
 # FAIL:  0
 # XPASS: 0
 # ERROR: 0
 =========================================================================

Fixes multipath-tcp#204.
Without 'sudo', mptcpd was started but the exit status was not checked.

So probably best to skip the test than marking it as PASS but not
checking anything.

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Without MPTCP, the sysctl commands fail and mptcpd always returns an
error because the kernel doesn't support MPTCP.

In other words, the test fails without MPTCP support, best to skip it.

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
@ossama-othman ossama-othman self-requested a review February 3, 2022 16:17
@ossama-othman ossama-othman added this to the Q1-2022 milestone Feb 3, 2022
@ossama-othman ossama-othman self-assigned this Feb 3, 2022
@coveralls
Copy link

coveralls commented Feb 3, 2022

Pull Request Test Coverage Report for Build 1789178453

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 53.266%

Totals Coverage Status
Change from base Build 1764368853: 0.0%
Covered Lines: 1060
Relevant Lines: 1990

💛 - Coveralls

Copy link
Member

@ossama-othman ossama-othman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks!

@ossama-othman ossama-othman merged commit 5d65e54 into multipath-tcp:master Feb 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.

3 participants