Skip to content

Commit

Permalink
Fix CI failing since rustc 1.65.0
Browse files Browse the repository at this point in the history
Two issues.

1. Backtrace
BT format changed in Rust 1.65, causing
bactrace_util::generate_backtraced_name to return an autogenerated name
created via uuidv4. However, changes applied since then have actually
made the happy path a hard requirement by expecting the resultant string
to be of the form r"(.*)::(.*)::.*". This was the case when the test
names were pulled from BT, but didn't work for autogenerated uuidv4 ones.

This patch removed the uuid name generation from backtrace_util, and
implemented it in mock_node in a different manner.

2. split-debuginfo=unpacked fails on Windows.
Cargo should not forward this option if not supported, but this does not
work in this case. Likely a regression in cargo-rustc interaction. For
now removed this flag with an appropriate remark to get the CI going.
  • Loading branch information
kamirr committed Nov 6, 2022
1 parent 4229a55 commit a104c0d
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 28 deletions.
8 changes: 6 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -227,5 +227,9 @@ web3 = { git = "https://github.com/golemfactory/rust-web3", branch = "update_eth

# Speed up builds on macOS (will be default in next rust version probably)
# https://jakedeichert.com/blog/reducing-rust-incremental-compilation-times-on-macos-by-70-percent/
[profile.dev]
split-debuginfo = "unpacked"
#
# TODO: reenable split-debuginfo.
# Commented out split-debuginfo makes Windows builds fail due to "`-Csplit-debuginfo=unpacked` is unstable on this platform."
# This should not be the case (cargo is meant to verify that this option is supported), but it is since version 1.65, I think.
# [profile.dev]
# split-debuginfo = "unpacked"
21 changes: 10 additions & 11 deletions core/market/src/testing/backtrace_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ fn adjust_backtrace_level(frames: &[backtrace::BacktraceFrame]) -> Option<usize>
}

fn get_symbol_at_level(bt: &backtrace::Backtrace, lvl: usize) -> Option<String> {
let frames = &bt.frames();
let frames = bt.frames();
match adjust_backtrace_level(frames) {
Some(adjustment) => {
let frame = &frames[lvl + adjustment];
Expand All @@ -34,18 +34,17 @@ fn get_symbol_at_level(bt: &backtrace::Backtrace, lvl: usize) -> Option<String>
None
}

pub fn generate_backtraced_name(level: Option<usize>) -> String {
pub fn generate_backtraced_name(level: Option<usize>) -> Option<String> {
let bt = backtrace::Backtrace::new();

// 0th element should be this function. We'd like to know the caller
if let Some(name) = get_symbol_at_level(&bt, level.unwrap_or(1)) {
let name = get_symbol_at_level(&bt, level.unwrap_or(1));

if let Some(name) = &name {
log::trace!("Generated name: {} level: {:?} BT: {:#?}", name, level, bt);
return name;
} else {
log::warn!("No backtrace support. bt={:#?}", bt);
}
let u4 = uuid::Uuid::new_v4().to_string();
log::error!(
"No backtrace support. Generating default name from UUIDv4. uuid4={}, bt={:#?}",
u4,
bt
);
u4

name
}
46 changes: 31 additions & 15 deletions core/market/src/testing/mock_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,14 @@ impl MockNodeKind {
}
}

fn testname_from_backtrace(bn: &str) -> String {
fn testname_from_backtrace(bn: &str) -> Option<String> {
log::info!("Test name to regex match: {}", &bn);
// Extract test name
let captures = Regex::new(r"(.*)::(.*)::.*").unwrap().captures(bn).unwrap();
let captures = Regex::new(r"(.*)::(.*)::.*").unwrap().captures(bn)?;
let filename = captures.get(1).unwrap().as_str().to_string();
let testname = captures.get(2).unwrap().as_str().to_string();

format!("{}.{}", filename, testname)
Some(format!("{}.{}", filename, testname))
}

impl MarketsNetwork {
Expand All @@ -122,19 +122,35 @@ impl MarketsNetwork {
pub async fn new(test_name: Option<&str>) -> Self {
std::env::set_var("RUST_LOG", "debug");
let _ = env_logger::builder().try_init();
// level 1 is this function.
// level 2 is <core::future::from_generator::GenFuture<T> as
// core::future::future::Future>::poll::XXX> (async)
// We want to know the caller.
let mut bn = crate::testing::backtrace_util::generate_backtraced_name(Some(3));
// Special case for mac&windows. Tests are run in adifferent way on those systems and we
// have to dive one less level down the stack to find the caller (test_* module).
if !bn.starts_with("test_") {
bn = crate::testing::backtrace_util::generate_backtraced_name(Some(2));
}
bn = testname_from_backtrace(&bn);

let test_name = test_name.unwrap_or(&bn).to_string();
let test_name_from_bt = || {
// level 1 is this function.
// level 2 is <core::future::from_generator::GenFuture<T> as
// core::future::future::Future>::poll::XXX> (async)
// We want to know the caller.
crate::testing::backtrace_util::generate_backtraced_name(Some(3))
.and_then(|name| {
// Special case for mac&windows. Tests are run in adifferent way on those systems and we
// have to dive one less level down the stack to find the caller (test_* module).
if !name.starts_with("test_") {
crate::testing::backtrace_util::generate_backtraced_name(Some(2))
} else {
Some(name)
}
})
.as_deref()
.and_then(testname_from_backtrace)
};

let unknown_test_name = || {
let nonce = rand::random::<u128>();
format!("unknown-test-{:#32x}", nonce)
};

let test_name = test_name
.map(String::from)
.or_else(test_name_from_bt)
.unwrap_or_else(unknown_test_name);
log::info!("Intializing MarketsNetwork. tn={}", test_name);

MockNet::default().bind_gsb();
Expand Down

0 comments on commit a104c0d

Please sign in to comment.