Skip to content

Commit 75d27f4

Browse files
committed
[WIP] option for store dir relative to the target dir
1 parent 44e109c commit 75d27f4

File tree

27 files changed

+325
-120
lines changed

27 files changed

+325
-120
lines changed

cargo-nextest/src/dispatch/execution.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use nextest_runner::{
2929
platform::BuildPlatforms,
3030
redact::Redactor,
3131
reporter::{
32+
EventAggregator,
3233
events::{FinalRunStats, RunStatsFailureKind},
3334
structured,
3435
},
@@ -387,11 +388,6 @@ impl BaseApp {
387388
let profile = config
388389
.profile(profile_name)
389390
.map_err(ExpectedError::profile_not_found)?;
390-
let store_dir = profile.store_dir();
391-
std::fs::create_dir_all(store_dir).map_err(|err| ExpectedError::StoreDirCreateError {
392-
store_dir: store_dir.to_owned(),
393-
err,
394-
})?;
395391
Ok(profile)
396392
}
397393
}
@@ -489,7 +485,7 @@ impl App {
489485
.base
490486
.load_runner(&binary_list.rust_build_meta.build_platforms);
491487
let profile =
492-
profile.apply_build_platforms(&binary_list.rust_build_meta.build_platforms);
488+
profile.into_evaluatable(&binary_list.rust_build_meta.build_platforms);
493489
let ctx = TestExecuteContext {
494490
profile_name: profile.name(),
495491
double_spawn,
@@ -545,7 +541,7 @@ impl App {
545541

546542
let double_spawn = self.base.load_double_spawn();
547543
let target_runner = self.base.load_runner(&build_platforms);
548-
let profile = profile.apply_build_platforms(&build_platforms);
544+
let profile = profile.into_evaluatable(&build_platforms);
549545
let ctx = TestExecuteContext {
550546
profile_name: profile.name(),
551547
double_spawn,
@@ -642,10 +638,16 @@ impl App {
642638

643639
let binary_list = self.base.build_binary_list()?;
644640
let build_platforms = &binary_list.rust_build_meta.build_platforms.clone();
641+
let target_dir = &binary_list.rust_build_meta.target_directory;
645642
let double_spawn = self.base.load_double_spawn();
646643
let target_runner = self.base.load_runner(build_platforms);
647644

648-
let profile = profile.apply_build_platforms(build_platforms);
645+
let profile = profile.into_evaluatable(build_platforms);
646+
647+
// Create the aggregator early so that any errors are reported before
648+
// running tests. This also creates the store directory.
649+
let aggregator = EventAggregator::new(&profile, target_dir)?;
650+
649651
let ctx = TestExecuteContext {
650652
profile_name: profile.name(),
651653
double_spawn,
@@ -739,6 +741,7 @@ impl App {
739741
&profile,
740742
&self.base.cargo_configs,
741743
output,
744+
aggregator,
742745
structured_reporter,
743746
);
744747

@@ -810,7 +813,7 @@ impl ArchiveApp {
810813
let profile = self
811814
.base
812815
.load_profile(&config)?
813-
.apply_build_platforms(build_platforms);
816+
.into_evaluatable(build_platforms);
814817
let redactor = if crate::output::should_redact() {
815818
Redactor::build_active(&binary_list.rust_build_meta)
816819
.with_path(output_file.to_path_buf(), "<archive-file>".to_owned())

cargo-nextest/src/errors.rs

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,10 @@ pub enum ExpectedError {
8282
#[from]
8383
err: ProfileNotFound,
8484
},
85-
#[error("failed to create store directory")]
86-
StoreDirCreateError {
87-
store_dir: Utf8PathBuf,
88-
#[source]
89-
err: std::io::Error,
85+
#[error("junit setup error")]
86+
JunitSetupError {
87+
#[from]
88+
err: JunitSetupError,
9089
},
9190
#[error("cargo config error")]
9291
CargoConfigError {
@@ -447,7 +446,7 @@ impl ExpectedError {
447446
| Self::SetCurrentDirFailed { .. }
448447
| Self::GetCurrentExeFailed { .. }
449448
| Self::ProfileNotFound { .. }
450-
| Self::StoreDirCreateError { .. }
449+
| Self::JunitSetupError { .. }
451450
| Self::RootManifestNotFound { .. }
452451
| Self::CargoConfigError { .. }
453452
| Self::TestFilterBuilderError { .. }
@@ -590,11 +589,8 @@ impl ExpectedError {
590589
);
591590
None
592591
}
593-
Self::StoreDirCreateError { store_dir, err } => {
594-
error!(
595-
"failed to create store dir at `{}`",
596-
store_dir.style(styles.bold)
597-
);
592+
Self::JunitSetupError { err } => {
593+
error!("failed to set up JUnit reporter");
598594
Some(err as &dyn Error)
599595
}
600596
Self::CargoConfigError { err } => {

nextest-runner/src/config/core/imp.rs

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ use crate::{
77
core::ConfigExperimental,
88
elements::{
99
ArchiveConfig, CustomTestGroup, DefaultJunitImpl, GlobalTimeout, JunitConfig,
10-
JunitImpl, LeakTimeout, MaxFail, RetryPolicy, SlowTimeout, TestGroup, TestGroupConfig,
11-
TestThreads, ThreadsRequired, deserialize_fail_fast, deserialize_leak_timeout,
12-
deserialize_retry_policy, deserialize_slow_timeout,
10+
JunitImpl, LeakTimeout, MaxFail, RetryPolicy, SlowTimeout, StoreConfigImpl, TestGroup,
11+
TestGroupConfig, TestThreads, ThreadsRequired, deserialize_fail_fast,
12+
deserialize_leak_timeout, deserialize_retry_policy, deserialize_slow_timeout,
1313
},
1414
overrides::{
1515
CompiledByProfile, CompiledData, CompiledDefaultFilter, DeserializedOverride,
@@ -794,19 +794,16 @@ impl NextestConfig {
794794
fn make_profile(&self, name: &str) -> Result<EarlyProfile<'_>, ProfileNotFound> {
795795
let custom_profile = self.inner.get_profile(name)?;
796796

797-
// The profile was found: construct it.
798-
let mut store_dir = self.workspace_root.join(&self.inner.store.dir);
799-
store_dir.push(name);
800-
801-
// Grab the compiled data as well.
797+
// Grab the compiled data.
802798
let compiled_data = match self.compiled.other.get(name) {
803799
Some(data) => data.clone().chain(self.compiled.default.clone()),
804800
None => self.compiled.default.clone(),
805801
};
806802

807803
Ok(EarlyProfile {
808804
name: name.to_owned(),
809-
store_dir,
805+
workspace_root: self.workspace_root.clone(),
806+
store: &self.inner.store,
810807
default_profile: &self.inner.default_profile,
811808
custom_profile,
812809
test_groups: &self.inner.test_groups,
@@ -871,7 +868,8 @@ pub(crate) struct FinalConfig {
871868
/// Returned by [`NextestConfig::profile`].
872869
pub struct EarlyProfile<'cfg> {
873870
name: String,
874-
store_dir: Utf8PathBuf,
871+
workspace_root: Utf8PathBuf,
872+
store: &'cfg StoreConfigImpl,
875873
default_profile: &'cfg DefaultProfileImpl,
876874
custom_profile: Option<&'cfg CustomProfileImpl>,
877875
test_groups: &'cfg BTreeMap<CustomTestGroup, TestGroupConfig>,
@@ -882,24 +880,18 @@ pub struct EarlyProfile<'cfg> {
882880
}
883881

884882
impl<'cfg> EarlyProfile<'cfg> {
885-
/// Returns the absolute profile-specific store directory.
886-
pub fn store_dir(&self) -> &Utf8Path {
887-
&self.store_dir
888-
}
889-
890883
/// Returns the global test group configuration.
891884
pub fn test_group_config(&self) -> &'cfg BTreeMap<CustomTestGroup, TestGroupConfig> {
892885
self.test_groups
893886
}
894887

895-
/// Applies build platforms to make the profile ready for evaluation.
888+
/// Converts this profile into an evaluatable profile by applying build
889+
/// platforms.
896890
///
897-
/// This is a separate step from parsing the config and reading a profile so that cargo-nextest
898-
/// can tell users about configuration parsing errors before building the binary list.
899-
pub fn apply_build_platforms(
900-
self,
901-
build_platforms: &BuildPlatforms,
902-
) -> EvaluatableProfile<'cfg> {
891+
/// This is a separate step from parsing the config and reading a profile so
892+
/// that cargo-nextest can tell users about configuration parsing errors
893+
/// before building the binary list.
894+
pub fn into_evaluatable(self, build_platforms: &BuildPlatforms) -> EvaluatableProfile<'cfg> {
903895
let compiled_data = self.compiled_data.apply_build_platforms(build_platforms);
904896

905897
let resolved_default_filter = {
@@ -921,7 +913,8 @@ impl<'cfg> EarlyProfile<'cfg> {
921913

922914
EvaluatableProfile {
923915
name: self.name,
924-
store_dir: self.store_dir,
916+
workspace_root: self.workspace_root,
917+
store: self.store,
925918
default_profile: self.default_profile,
926919
custom_profile: self.custom_profile,
927920
scripts: self.scripts,
@@ -934,11 +927,12 @@ impl<'cfg> EarlyProfile<'cfg> {
934927

935928
/// A configuration profile for nextest. Contains most configuration used by the nextest runner.
936929
///
937-
/// Returned by [`EarlyProfile::apply_build_platforms`].
930+
/// Returned by [`EarlyProfile::into_evaluatable`].
938931
#[derive(Clone, Debug)]
939932
pub struct EvaluatableProfile<'cfg> {
940933
name: String,
941-
store_dir: Utf8PathBuf,
934+
workspace_root: Utf8PathBuf,
935+
store: &'cfg StoreConfigImpl,
942936
default_profile: &'cfg DefaultProfileImpl,
943937
custom_profile: Option<&'cfg CustomProfileImpl>,
944938
test_groups: &'cfg BTreeMap<CustomTestGroup, TestGroupConfig>,
@@ -958,8 +952,18 @@ impl<'cfg> EvaluatableProfile<'cfg> {
958952
}
959953

960954
/// Returns the absolute profile-specific store directory.
961-
pub fn store_dir(&self) -> &Utf8Path {
962-
&self.store_dir
955+
///
956+
/// The target directory must be provided to resolve the store directory
957+
/// when it is configured relative to the target directory.
958+
///
959+
/// The store directory might not exist yet. The caller is responsible for
960+
/// creating it.
961+
pub fn store_dir(&self, target_dir: &Utf8Path) -> Utf8PathBuf {
962+
let mut store_dir = self
963+
.store
964+
.resolve_store_dir(&self.workspace_root, target_dir);
965+
store_dir.push(&self.name);
966+
store_dir
963967
}
964968

965969
/// Returns the context in which to evaluate filtersets.
@@ -1102,7 +1106,6 @@ impl<'cfg> EvaluatableProfile<'cfg> {
11021106
/// Returns the JUnit configuration for this profile.
11031107
pub fn junit(&self) -> Option<JunitConfig<'cfg>> {
11041108
JunitConfig::new(
1105-
self.store_dir(),
11061109
self.custom_profile.map(|p| &p.junit),
11071110
&self.default_profile.junit,
11081111
)
@@ -1210,12 +1213,6 @@ impl NextestConfigDeserialize {
12101213
}
12111214
}
12121215

1213-
#[derive(Clone, Debug, Deserialize)]
1214-
#[serde(rename_all = "kebab-case")]
1215-
struct StoreConfigImpl {
1216-
dir: Utf8PathBuf,
1217-
}
1218-
12191216
#[derive(Clone, Debug)]
12201217
pub(in crate::config) struct DefaultProfileImpl {
12211218
default_filter: String,

nextest-runner/src/config/core/tool_config.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ mod tests {
224224
let default_profile = config
225225
.profile(NextestConfig::DEFAULT_PROFILE)
226226
.expect("default profile is present")
227-
.apply_build_platforms(&build_platforms());
227+
.into_evaluatable(&build_platforms());
228228
// This is present in .config/nextest.toml and is the highest priority
229229
assert_eq!(default_profile.retries(), RetryPolicy::new_without_delay(3));
230230

@@ -288,7 +288,7 @@ mod tests {
288288
let tool_profile = config
289289
.profile("tool")
290290
.expect("tool profile is present")
291-
.apply_build_platforms(&build_platforms());
291+
.into_evaluatable(&build_platforms());
292292
assert_eq!(tool_profile.retries(), RetryPolicy::new_without_delay(12));
293293
assert_eq!(
294294
tool_profile.settings_for(&test_foo_query).retries(),
@@ -309,7 +309,7 @@ mod tests {
309309
let tool2_profile = config
310310
.profile("tool2")
311311
.expect("tool2 profile is present")
312-
.apply_build_platforms(&build_platforms());
312+
.into_evaluatable(&build_platforms());
313313
assert_eq!(tool2_profile.retries(), RetryPolicy::new_without_delay(18));
314314
assert_eq!(
315315
tool2_profile.settings_for(&test_foo_query).retries(),

nextest-runner/src/config/elements/archive.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ mod tests {
303303
config
304304
.profile("default")
305305
.expect("default profile exists")
306-
.apply_build_platforms(&build_platforms())
306+
.into_evaluatable(&build_platforms())
307307
.archive_config(),
308308
&default_config,
309309
"default matches"
@@ -313,7 +313,7 @@ mod tests {
313313
config
314314
.profile("profile1")
315315
.expect("profile exists")
316-
.apply_build_platforms(&build_platforms())
316+
.into_evaluatable(&build_platforms())
317317
.archive_config(),
318318
&ArchiveConfig {
319319
include: vec![ArchiveInclude {
@@ -330,7 +330,7 @@ mod tests {
330330
config
331331
.profile("profile2")
332332
.expect("default profile exists")
333-
.apply_build_platforms(&build_platforms())
333+
.into_evaluatable(&build_platforms())
334334
.archive_config(),
335335
&ArchiveConfig { include: vec![] },
336336
"profile2 matches"
@@ -340,7 +340,7 @@ mod tests {
340340
config
341341
.profile("profile3")
342342
.expect("default profile exists")
343-
.apply_build_platforms(&build_platforms())
343+
.into_evaluatable(&build_platforms())
344344
.archive_config(),
345345
&default_config,
346346
"profile3 matches"

nextest-runner/src/config/elements/global_timeout.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ mod tests {
8787
nextest_config
8888
.profile("default")
8989
.expect("default profile should exist")
90-
.apply_build_platforms(&build_platforms())
90+
.into_evaluatable(&build_platforms())
9191
.global_timeout(),
9292
expected_default,
9393
);
@@ -97,7 +97,7 @@ mod tests {
9797
nextest_config
9898
.profile("ci")
9999
.expect("ci profile should exist")
100-
.apply_build_platforms(&build_platforms())
100+
.into_evaluatable(&build_platforms())
101101
.global_timeout(),
102102
expected_ci,
103103
);

nextest-runner/src/config/elements/junit.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use serde::Deserialize;
99
/// Returned by an [`EvaluatableProfile`](crate::config::core::EvaluatableProfile).
1010
#[derive(Clone, Debug)]
1111
pub struct JunitConfig<'cfg> {
12+
// Stored as a relative path; joined with store_dir when accessed.
1213
path: Utf8PathBuf,
1314
report_name: &'cfg str,
1415
store_success_output: bool,
@@ -17,7 +18,6 @@ pub struct JunitConfig<'cfg> {
1718

1819
impl<'cfg> JunitConfig<'cfg> {
1920
pub(in crate::config) fn new(
20-
store_dir: &Utf8Path,
2121
custom_data: Option<&'cfg JunitImpl>,
2222
default_data: &'cfg DefaultJunitImpl,
2323
) -> Option<Self> {
@@ -27,7 +27,6 @@ impl<'cfg> JunitConfig<'cfg> {
2727
.as_deref();
2828

2929
path.map(|path| {
30-
let path = store_dir.join(path);
3130
let report_name = custom_data
3231
.and_then(|custom| custom.report_name.as_deref())
3332
.unwrap_or(&default_data.report_name);
@@ -38,7 +37,7 @@ impl<'cfg> JunitConfig<'cfg> {
3837
.and_then(|custom| custom.store_failure_output)
3938
.unwrap_or(default_data.store_failure_output);
4039
Self {
41-
path,
40+
path: path.to_owned(),
4241
report_name,
4342
store_success_output,
4443
store_failure_output,
@@ -47,8 +46,10 @@ impl<'cfg> JunitConfig<'cfg> {
4746
}
4847

4948
/// Returns the absolute path to the JUnit report.
50-
pub fn path(&self) -> &Utf8Path {
51-
&self.path
49+
///
50+
/// The store directory must be provided to compute the full path.
51+
pub fn path(&self, store_dir: &Utf8Path) -> Utf8PathBuf {
52+
store_dir.join(&self.path)
5253
}
5354

5455
/// Returns the name of the JUnit report.

0 commit comments

Comments
 (0)