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.

Sign up for GitHub

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(iroh-net): Fix some flaky magicsock tests #2034

Merged
merged 10 commits into from
Feb 21, 2024
Merged

fix(iroh-net): Fix some flaky magicsock tests #2034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ae1448c
Facelift the magicsock roundtrip test
flub Feb 20, 2024
7515f45
Merge branch 'main' into flub/test-magicsock-roundtrip
flub Feb 20, 2024
176b9ee
move setup_multithreaded_logging to iroh test
flub Feb 20, 2024
175e468
some logging tweaking, i love tweaking logging
flub Feb 20, 2024
cd26ca3
fix: make mesh_stacks reliable
flub Feb 20, 2024
45a1561
clippy
flub Feb 20, 2024
f6d540f
move awaiting meshed-stack readyness to mesh_stacks
flub Feb 21, 2024
93d1b31
get network changes test back as well
flub Feb 21, 2024
891a6e2
give me better debug information for when this test hangs
flub Feb 21, 2024
2f5e8f0
Let's mark this flaky again to get the other fixes in
flub Feb 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,15 @@ jobs:
if: matrix.target != 'i686-unknown-linux-gnu'
run: cross build --all --target ${{ matrix.target }}
env:
RUST_LOG: ${{ runner.debug && 'DEBUG' || 'INFO'}}
RUST_LOG: ${{ runner.debug && 'TRACE' || 'DEBUG'}}

- name: test
# cross tests are currently broken vor armv7 and aarch64
# see https://github.com/cross-rs/cross/issues/1311
if: matrix.target == 'i686-unknown-linux-gnu'
run: cross test --all --target ${{ matrix.target }} -- --test-threads=12
env:
RUST_LOG: ${{ runner.debug && 'DEBUG' || 'INFO'}}
RUST_LOG: ${{ runner.debug && 'TRACE' || 'DEBUG' }}

check_fmt_and_docs:
timeout-minutes: 30
Expand Down
8 changes: 5 additions & 3 deletions .github/workflows/tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ jobs:
cargo check -p $i $FEATURES --lib --bins
done
env:
RUST_LOG: ${{ runner.debug && 'DEBUG' || 'INFO'}}
RUST_LOG: ${{ runner.debug && 'TRACE' || 'DEBUG'}}

- name: build tests
run: |
Expand All @@ -109,12 +109,14 @@ jobs:
run: |
cargo nextest run --workspace ${{ env.FEATURES }} --lib --bins --tests --run-ignored ${{ inputs.flaky && 'all' || 'default' }} --no-fail-fast
env:
RUST_LOG: "TRACE"
RUST_LOG: ${{ runner.debug && 'TRACE' || 'DEBUG'}}

- name: doctests
if: ${{ (! inputs.flaky) && matrix.features == 'all' }}
run: |
if [ -n "${{ runner.debug }}" ]; then
export RUST_LOG=TRACE
else
export RUST_LOG=DEBUG
fi
cargo test --workspace --all-features --doc
Expand Down Expand Up @@ -195,4 +197,4 @@ jobs:
run: |
cargo nextest run --workspace ${{ env.FEATURES }} --lib --bins --tests --target ${{ matrix.target }} --run-ignored ${{ inputs.flaky && 'all' || 'default' }} --no-fail-fast
env:
RUST_LOG: "TRACE"
RUST_LOG: ${{ runner.debug && 'TRACE' || 'DEBUG'}}
6 changes: 5 additions & 1 deletion iroh-net/src/key.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,11 @@ impl Debug for PublicKey {

impl Display for PublicKey {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", base32::fmt(self.as_bytes()))
if f.alternate() {
Copy link
Contributor

Choose a reason for hiding this comment

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

when/how is this triggered?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When you use a # formatter: {:#} (or {:#?} for Debug)

Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn’t it be the other way around then,? # prints longer more detailed messages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could be rather tempted to switch this around and do the short format by default and the long format for alternate. That would reduce rather a lot of verbosity in our code.

Copy link
Contributor

Choose a reason for hiding this comment

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

the issue is that, even externally, we rely on to_string() roundtriping

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, it'd probably break things if this can't be round-tripped. so maybe we'll have to settle for this.

write!(f, "{}", base32::fmt_short(self.as_bytes()))
} else {
write!(f, "{}", base32::fmt(self.as_bytes()))
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions iroh-net/src/magic_endpoint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
derp::{DerpMap, DerpMode, DerpUrl},
key::{PublicKey, SecretKey},
magicsock::{self, Discovery, MagicSock},
tls,
tls, NodeId,
};

pub use super::magicsock::EndpointInfo as ConnectionInfo;
Expand Down Expand Up @@ -321,7 +321,7 @@ impl MagicEndpoint {
}

/// Get the node id of this endpoint.
pub fn node_id(&self) -> PublicKey {
pub fn node_id(&self) -> NodeId {
self.secret_key.public()
}

Expand Down
Loading
Loading