Skip to content

Commit

Permalink
Fix bug leading to doctests not recognized when added
Browse files Browse the repository at this point in the history
  • Loading branch information
@hundsdor
hundsdor committed Oct 2, 2024
1 parent 57b8e2e commit 0636b1e
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 75 deletions.
43 changes: 26 additions & 17 deletions src/bin/cargo-rustyrts/commands/dynamic.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ use internment::{Arena, ArenaIntern};

use itertools::Itertools;
use rustyrts::{
callbacks_shared::DOCTEST_PREFIX,
constants::{
ENDING_CHANGES, ENDING_PROCESS_TRACE, ENV_COMPILE_MODE, ENV_DOCTESTED,
ENV_ONLY_INSTRUMENTATION, ENV_TARGET, ENV_TARGET_DIR,
Expand All @@ -39,7 +38,7 @@ use test::{test::parse_opts, TestOpts};
use tracing::debug;

use crate::{
commands::{convert_doctest_name, TestInfo},
commands::{DoctestName, TestInfo},
ops::PreciseExecutor,
target_hash::get_target_hash,
};
Expand Down Expand Up @@ -474,23 +473,32 @@ impl<'arena, 'context> Selector<'context> for DynamicSelector<'arena, 'context>
);

let mut tests_found = HashSet::new();
let tests = tests.into_iter().map(|s| convert_doctest_name(&s)).unique();
let tests = tests
.into_iter()
.map(|s| DoctestName::new(s))
.map(|t| (t.cache_name(), t.trimmed_name().to_string(), t.fn_name()))
.unique();

let no_traces = {
let mut map_no_traces = HashMap::new();

for (trimmed, fn_name) in tests {
let dependency_unit = DependencyUnit::DoctestUnit(unit, fn_name.clone());
let changed = self.changed_nodes(dependency_unit).clone();
for (cache_name, trimmed_name, fn_name) in tests {
let test = self.arena.intern(cache_name.clone());

let path = CacheKind::Dynamic.map(self.target_dir.to_path_buf());
let dependency_unit = DependencyUnit::DoctestUnit(unit, cache_name.clone());
let changed = self.changed_nodes(dependency_unit).clone();

let interned = self.arena.intern(trimmed.clone());
print_doctest_stats(
shell,
"Doc test ".to_string() + &trimmed_name,
&changed,
)
.unwrap();

tests_found.insert(interned);
let path = CacheKind::Dynamic.map(self.target_dir.to_path_buf());

let descr = CacheFileDescr::new(
fn_name.as_str(),
&cache_name,
None,
None,
None,
Expand All @@ -505,24 +513,25 @@ impl<'arena, 'context> Selector<'context> for DynamicSelector<'arena, 'context>
.map(|l| Arena::<String>::intern(self.arena, l))
.collect::<HashSet<_>>()
}) {
traced_tests.insert(interned);
traced_tests.insert(test);
let mut intersection = traces.intersection(&changed).peekable();
if intersection.peek().is_some() {
debug!(
"Found non-empty intersection: {:?}",
intersection.collect::<Vec<_>>()
);
affected_tests.push(interned.to_string());
affected_tests.push(trimmed_name);
}
} else {
let name = DOCTEST_PREFIX.to_string() + &fn_name;
debug!("Found no traces");
map_no_traces
.insert(interned, Arena::<String>::intern(self.arena, name));
map_no_traces.insert(
trimmed_name,
Arena::<String>::intern(self.arena, fn_name.clone()),
);
changed_nodes.extend(changed.clone());
}
print_doctest_stats(shell, "Doc test ".to_string() + &trimmed, &changed)
.unwrap();

tests_found.insert(test);
}

map_no_traces
Expand Down
42 changes: 35 additions & 7 deletions src/bin/cargo-rustyrts/commands/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use cargo::{
use cargo_util::ProcessBuilder;
use internment::{Arena, ArenaIntern};
use rustyrts::{
callbacks_shared::DOCTEST_PREFIX,
constants::ENV_RETEST_ALL,
fs_utils::{CacheFileDescr, CacheFileKind, CacheKind},
};
Expand Down Expand Up @@ -285,11 +286,38 @@ Proceed with caution!!!
crate::ops::run_tests(&ws, &ops, &test_args, selection)
}

pub fn convert_doctest_name(test_name: &str) -> (String, String) {
let (trimmed, _) = test_name.split_once(" - ").unwrap();
let fn_name = trimmed
.chars()
.map(|c| if c.is_ascii_alphanumeric() { c } else { '_' })
.collect::<String>();
(trimmed.to_string(), fn_name)
#[derive(Debug)]
pub(crate) struct DoctestName {
inner: String,
}

impl DoctestName {
pub(crate) fn new(inner: String) -> Self {
debug_assert!(inner.contains(" - "));
Self { inner }
}

pub(crate) fn full_name(&self) -> String {
self.inner.split_once(" (line").unwrap().0.to_owned()
}

pub(crate) fn trimmed_name(&self) -> String {
self.inner.split_once(" - ").unwrap().0.to_owned()
}

pub(crate) fn cache_name(&self) -> String {
self.trimmed_name()
.chars()
.map(|c| if c.is_ascii_alphanumeric() { c } else { '_' })
.collect::<String>()
}

pub(crate) fn fn_name(&self) -> String {
DOCTEST_PREFIX.to_string()
+ &self
.trimmed_name()
.chars()
.map(|c| if c.is_ascii_alphanumeric() { c } else { '_' })
.collect::<String>()
}
}
27 changes: 13 additions & 14 deletions src/bin/cargo-rustyrts/commands/static.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ use cargo::{

use cargo_util::ProcessBuilder;
use internment::{Arena, ArenaIntern};
use itertools::Itertools;
use rustyrts::{
callbacks_shared::DOCTEST_PREFIX,
constants::{ENDING_CHANGES, ENV_COMPILE_MODE, ENV_DOCTESTED, ENV_TARGET, ENV_TARGET_DIR},
fs_utils::{CacheFileDescr, CacheFileKind, CacheKind},
static_rts::graph::{serialize::ArenaDeserializable, DependencyGraph},
};
use tracing::trace;

use crate::{commands::convert_doctest_name, ops::PreciseExecutor, target_hash::get_target_hash};
use crate::{commands::DoctestName, ops::PreciseExecutor, target_hash::get_target_hash};

use super::{
cache::HashCache, DependencyUnit, PreciseSelectionMode, SelectionContext, SelectionMode,
Expand Down Expand Up @@ -299,12 +299,7 @@ impl<'arena: 'context, 'context> StaticSelector<'arena, 'context> {
"Did not find dependency graph for crate {:?} in mode {:?}\nTried reading from {:?}",
crate_name, compile_mode, graph_path
);
let mut graph = DependencyGraph::new(arena);
if let Some(doctest_name) = maybe_doctest_name {
let entry_name = DOCTEST_PREFIX.to_string() + doctest_name;
graph.add_edge(doctest_name.to_string(), entry_name, rustyrts::static_rts::graph::EdgeType::Trimmed);
}
graph
DependencyGraph::new(arena)
}, |s| DependencyGraph::deserialize(arena, &s).unwrap());

if maybe_doctest_name.is_some() && graph_path.is_file() {
Expand Down Expand Up @@ -524,25 +519,29 @@ impl<'arena, 'context> Selector<'context> for StaticSelector<'arena, 'context> {
);

let mut tests_found = HashSet::new();
let tests = tests
.into_iter()
.map(|s| DoctestName::new(s))
.map(|t| (t.cache_name(), t.trimmed_name().to_string(), t.fn_name()))
.unique();

for test in &tests {
let (trimmed, test_path) = convert_doctest_name(test);
let test = self.arena.intern(test_path.clone());
for (cache_name, trimmed_name, fn_name) in tests {
let test = self.arena.intern(fn_name.clone());

let dependency_unit = DependencyUnit::DoctestUnit(unit, test_path);
let dependency_unit = DependencyUnit::DoctestUnit(unit, cache_name);
let (changed, locally, reachable) =
self.reachble_and_changed_nodes(dependency_unit);

print_doctest_stats(
shell,
"Doc test ".to_string() + &trimmed,
"Doc test ".to_string() + &trimmed_name,
&changed,
&reachable,
)
.unwrap();

if locally.contains(&test) {
affected_tests.push(trimmed);
affected_tests.push(trimmed_name);
}
tests_found.insert(test);
}
Expand Down
47 changes: 28 additions & 19 deletions src/bin/cargo-rustyrts/doctest_rts.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::{
collections::HashSet,
collections::{HashMap, HashSet},
ffi::OsString,
hash::DefaultHasher,
hash::Hasher,
Expand All @@ -18,12 +18,12 @@ use cargo::{
use cargo_util::ProcessBuilder;
use itertools::Itertools;
use rustyrts::{
callbacks_shared::{ChecksumsCallback, CompileMode, RTSContext, Target, DOCTEST_PREFIX},
callbacks_shared::{ChecksumsCallback, CompileMode, RTSContext, Target},
checksums::Checksums,
fs_utils::{CacheKind, ChecksumKind},
};

use crate::commands::convert_doctest_name;
use crate::commands::DoctestName;

struct DoctestAnalysis {
path: PathBuf,
Expand Down Expand Up @@ -143,16 +143,25 @@ pub(crate) fn run_analysis_doctests(
}

{
let grouped = test_names
.iter()
.sorted()
.group_by(|test| convert_doctest_name(test).1);
let grouped = {
let mut groups = HashMap::new();

let mut tests = Vec::new();
for test in &test_names {
let doctest_name = DoctestName::new(test.clone());
let key = (doctest_name.fn_name(), doctest_name.cache_name());

if groups.get(&key).is_none() {
groups.insert(key.clone(), Vec::new()).unwrap_or_default();
}
groups.get_mut(&key).unwrap().push(doctest_name);
}

groups
};

for (test_fn, names) in grouped.into_iter() {
let doctest_name = Some(test_fn.clone());
let mut tests = Vec::new();

for ((fn_name, cache_name), names) in grouped {
let compile_mode = CompileMode::try_from(format!("{:?}", unit.mode).as_str()).unwrap();
let target =
Target::try_from(format!("{}", unit.target.kind().description()).as_str()).unwrap();
Expand All @@ -163,7 +172,8 @@ pub(crate) fn run_analysis_doctests(
unit.target.crate_name(),
compile_mode,
target,
doctest_name,
Some(cache_name.clone()),
Some(fn_name.clone()),
),
};

Expand All @@ -183,7 +193,7 @@ pub(crate) fn run_analysis_doctests(
.get_or_init(|| old_checksums_const);
}

tests.push((analysis, names));
tests.push((fn_name, analysis, names));
}

{
Expand All @@ -197,32 +207,31 @@ pub(crate) fn run_analysis_doctests(
let _ = p.output();
}

for (analysis, names) in &mut tests {
for (fn_name, analysis, names) in &mut tests {
let mut checksums = Checksums::new();

// We add a hash of the names of all tests, to recognize newly added or removed compile-fail tests
let name = DOCTEST_PREFIX.to_string() + analysis.context.doctest_name.as_ref().unwrap();
let hash = {
let mut hasher = DefaultHasher::new();
for name in names {
hasher.write(name.as_bytes());
hasher.write(name.full_name().as_bytes());
}
let value = hasher.finish();

(value, value)
};

if checksums.get(&name).is_none() {
if checksums.get(fn_name).is_none() {
checksums
.insert(name.clone(), HashSet::new())
.insert(fn_name.clone(), HashSet::new())
.unwrap_or_default();
}
checksums.get_mut(&name).unwrap().insert(hash);
checksums.get_mut(fn_name).unwrap().insert(hash);

analysis.export_checksums(ChecksumKind::Checksum, &checksums, true);
}

for (analysis, _count) in &mut tests {
for (_fn_name, analysis, _count) in &mut tests {
let checksums = analysis.import_checksums(ChecksumKind::Checksum, false);
let checksums_vtbl = analysis.import_checksums(ChecksumKind::VtblChecksum, false);
let checksums_const = analysis.import_checksums(ChecksumKind::ConstChecksum, false);
Expand Down
Loading

0 comments on commit 0636b1e

Please sign in to comment.