Skip to content

Improve integration tests interface-ready waiting logic (LP: #1922126)#204

Merged
slyon merged 7 commits into
masterfrom
slyon/tests-wait-per-interface
Jul 5, 2021
Merged

Improve integration tests interface-ready waiting logic (LP: #1922126)#204
slyon merged 7 commits into
masterfrom
slyon/tests-wait-per-interface

Conversation

@slyon

@slyon slyon commented Apr 7, 2021

Copy link
Copy Markdown
Contributor

Description

This is a big refactoring of the integration test suite, which should make it much faster and more stable in different testing environments (e.g. armhf LXD containers).

The main change is in tests/integration/base.py and changes the waiting logic to decide when an given interface is readily configured. Previously, we were relying on /lib/systemd/systemd-networkd-wait-online to tell us magically when the networking is fully configured. This does not properly work for interfaces controlled by NetworkManager and is only watching the "general" networking state (i.e. whenever the first interface is up and running).

Waiting only on an arbitrary "first" network interface to be ready leads to races as we use asserts on many individual interfaces, which might not be ready at that time. So with this change the generate_and_settle(self, wait_interfaces=None): method got a new wait_interfaces argument, where the developer needs to specify each interface to be tested in a given test case. Furthermore, the developer can wait for a specified state (i.e. DHCP4/6 address assigned), to account for waiting on asynchronous background tasks, e.g.:

self.generate_and_settle([self.dev_e_client, 'eth43', self.state_dhcp4('mybond')])

All existing integration tests have been adopted to specify their interfaces-under-test in generate_and_settle(). Additionally, some logging was added to always print the name of the currently waited on interface and one dot per second (up to 60s), to see how long it takes to become ready (or times out), especially on slower architectures.

Checklist

@slyon slyon force-pushed the slyon/tests-wait-per-interface branch 2 times, most recently from 2674505 to 3bd8f60 Compare April 7, 2021 13:31
@slyon slyon changed the title Improve integration tests interface-ready waiting logic Improve integration tests interface-ready waiting logic (LP: #1922126) May 5, 2021
@slyon slyon force-pushed the slyon/tests-wait-per-interface branch from 16a4152 to 0212e82 Compare June 15, 2021 15:39
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #204 (16a4152) into master (2a7fc68) will increase coverage by 0.08%.
The diff coverage is n/a.

❗ Current head 16a4152 differs from pull request most recent head 0212e82. Consider uploading reports for the commit 0212e82 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #204      +/-   ##
==========================================
+ Coverage   98.92%   99.00%   +0.08%     
==========================================
  Files          54       54              
  Lines        8428     8324     -104     
==========================================
- Hits         8337     8241      -96     
+ Misses         91       83       -8     
Impacted Files Coverage Δ
src/nm.c 99.46% <0.00%> (-0.01%) ⬇️
src/dbus.c 100.00% <0.00%> (ø)
src/parse.c 100.00% <0.00%> (ø)
src/networkd.c 100.00% <0.00%> (ø)
tests/dbus/test_dbus.py 100.00% <0.00%> (ø)
netplan/cli/commands/set.py 100.00% <0.00%> (ø)
netplan/cli/commands/apply.py 100.00% <0.00%> (ø)
tests/generator/test_common.py 100.00% <0.00%> (ø)
tests/generator/test_errors.py 100.00% <0.00%> (ø)
netplan/cli/commands/generate.py 100.00% <0.00%> (+24.24%) ⬆️

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 2a7fc68...0212e82. Read the comment docs.

@slyon slyon marked this pull request as ready for review June 15, 2021 15:56
@slyon slyon requested a review from sil2100 June 15, 2021 15:57

@sil2100 sil2100 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, I actually forgot to approve this one earlier. So yeah, this looks good and I like this wait-logic much more than what we had previously.

@sil2100

sil2100 commented Jul 5, 2021

Copy link
Copy Markdown
Contributor

Just curious: codecov seems to say there's less coverage this time, is that true or is it just getting confused?

@slyon

slyon commented Jul 5, 2021

Copy link
Copy Markdown
Contributor Author

\o/ Thanks for the review and approval! The make check-coverage command shows a 100% coverage, so that must be a CodeCov hiccup. I'll be doing a (clean) rebase and merge.

@slyon slyon force-pushed the slyon/tests-wait-per-interface branch from 0212e82 to 8a9469b Compare July 5, 2021 09:47
@slyon slyon merged commit 9ae9eb7 into master Jul 5, 2021
@slyon slyon deleted the slyon/tests-wait-per-interface branch November 14, 2024 12:00
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