Skip to content

Commit 0f6c63c

Browse files
rydrmanrydrman
authored
Merge pull request #1219 from spkenv/fix-tests-on-arch
Fix host options related ls tests that are failing on Arch Linux
2 parents 70a531d + 5ac7ebf commit 0f6c63c

File tree

8 files changed

+138
-102
lines changed

8 files changed

+138
-102
lines changed

‎Cargo.lock‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/spk-cli/cmd-build/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ spk-cmd-make-source = { workspace = true }
3030

3131
[dev-dependencies]
3232
rstest = { workspace = true }
33+
serial_test = { workspace = true }
3334
spk-schema = { workspace = true }
3435
spk-storage = { workspace = true }
3536
tempfile = { workspace = true }

‎crates/spk-cli/cmd-build/src/cmd_build_test/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,7 @@ build:
611611
// this case passes because host options override default values, and the
612612
// provided host option value of "linux" is a valid choice.
613613
#[case::non_empty_value_for_host_option_bad_value_succeeds_with_host_options_enabled("os", "beos", &["linux", "windows"], true)]
614+
#[serial_test::serial(host_options)]
614615
#[tokio::test]
615616
async fn test_options_with_choices_and_empty_values(
616617
tmpdir: tempfile::TempDir,

‎crates/spk-cli/cmd-build/src/cmd_build_test/variant_filter.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,7 @@ build:
661661
}
662662

663663
#[rstest]
664+
#[serial_test::serial(host_options)]
664665
#[tokio::test]
665666
async fn test_build_filters_variants_based_on_host_opts(tmpdir: tempfile::TempDir) {
666667
let _rt = spfs_runtime().await;

‎crates/spk-cli/group2/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,5 @@ tracing = { workspace = true }
3636
futures = { workspace = true }
3737
relative-path = { workspace = true }
3838
rstest = { workspace = true }
39+
serial_test = { workspace = true }
3940
tempfile = { workspace = true }

‎crates/spk-cli/group2/src/cmd_ls_test.rs‎

Lines changed: 125 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use spk_schema::foundation::ident_component::Component;
1313
use spk_schema::name::OptName;
1414
use spk_schema::recipe;
1515
use spk_solve::option_map::HOST_OPTIONS;
16-
use spk_solve::spec;
16+
use spk_solve::{option_map, spec};
1717
use spk_storage::RepositoryHandle;
1818
use spk_storage::fixtures::*;
1919

@@ -98,121 +98,147 @@ async fn test_ls_shows_remote_packages_with_no_host() {
9898
/// `spk ls` is expected to list packages in the configured remote
9999
/// repositories that match the default filter for the current host
100100
#[tokio::test]
101+
#[serial_test::serial(host_options)]
101102
async fn test_ls_shows_remote_packages_with_host_default_filter() {
102103
let mut rt = spfs_runtime().await;
103-
let remote_repo = spfsrepo().await;
104-
105-
// Populate the "origin" repo with one package.
106-
// The "local" repo is empty.
107-
108-
rt.add_remote_repo(
109-
"origin",
110-
Remote::Address(RemoteAddress {
111-
address: remote_repo.address().clone(),
112-
}),
113-
)
114-
.unwrap();
115104

116-
let recipe = recipe!({"pkg": "my-pkg/1.0.0"});
117-
remote_repo.publish_recipe(&recipe).await.unwrap();
118-
let host_options = HOST_OPTIONS.get().unwrap();
119-
let os_id = host_options.get(OptName::distro()).unwrap();
120-
121-
// Build with all matching host options
122-
let spec = spec!({"pkg": "my-pkg/1.0.0/BGSHW3CN",
123-
"build": {
124-
"options":
125-
[
126-
{"var": format!("{}/{}", OptName::distro(), host_options.get(OptName::distro()).unwrap()) },
127-
{"var": format!("{}/{}", OptName::os(), host_options.get(OptName::os()).unwrap()) },
128-
{"var": format!("{}/{}", OptName::arch(), host_options.get(OptName::arch()).unwrap()) },
129-
{"var": format!("{}/{}", os_id, host_options.get(os_id).unwrap()) }
130-
]
131-
}});
132-
remote_repo
133-
.publish_package(
134-
&spec,
135-
&vec![(Component::Run, empty_layer_digest())]
136-
.into_iter()
137-
.collect(),
105+
// Set the host options to make the test deterministic wherever it runs.
106+
// There isn't anything meaningful about the values picked for this.
107+
HOST_OPTIONS.scoped_options(Ok(option_map! {
108+
"os" => "linux",
109+
"arch" => "x86_64",
110+
"distro" => "rocky",
111+
"rocky" => "9.5",
112+
}), async move {
113+
let remote_repo = spfsrepo().await;
114+
115+
// Populate the "origin" repo with one package.
116+
// The "local" repo is empty.
117+
118+
rt.add_remote_repo(
119+
"origin",
120+
Remote::Address(RemoteAddress {
121+
address: remote_repo.address().clone(),
122+
}),
138123
)
139-
.await
140124
.unwrap();
141125

142-
let mut opt = Opt::try_parse_from(["ls", "--host"]).unwrap();
143-
opt.ls.run().await.unwrap();
144-
assert_eq!(opt.ls.output.vec.len(), 1);
126+
let recipe = recipe!({"pkg": "my-pkg/1.0.0"});
127+
remote_repo.publish_recipe(&recipe).await.unwrap();
128+
let host_options = HOST_OPTIONS.get().unwrap();
129+
let os_id = host_options.get(OptName::distro()).unwrap();
130+
131+
// Build with all matching host options
132+
let spec = spec!({"pkg": "my-pkg/1.0.0/BGSHW3CN",
133+
"build": {
134+
"options":
135+
[
136+
{"var": format!("{}/{}", OptName::distro(), host_options.get(OptName::distro()).unwrap()) },
137+
{"var": format!("{}/{}", OptName::os(), host_options.get(OptName::os()).unwrap()) },
138+
{"var": format!("{}/{}", OptName::arch(), host_options.get(OptName::arch()).unwrap()) },
139+
{"var": format!("{}/{}", os_id, host_options.get(os_id).unwrap()) }
140+
]
141+
}});
142+
remote_repo
143+
.publish_package(
144+
&spec,
145+
&vec![(Component::Run, empty_layer_digest())]
146+
.into_iter()
147+
.collect(),
148+
)
149+
.await
150+
.unwrap();
151+
152+
let mut opt = Opt::try_parse_from(["ls", "--host"]).unwrap();
153+
opt.ls.run().await.unwrap();
154+
assert_eq!(opt.ls.output.vec.len(), 1);
155+
156+
Ok::<_, ()>(())
157+
}).await.unwrap();
145158
}
146159

147160
/// `spk ls` is expected to list packages in the configured remote
148161
/// repositories that match the default filter for the current host
149162
#[tokio::test]
163+
#[serial_test::serial(host_options)]
150164
async fn test_ls_shows_remote_packages_with_host_default_filter_multiple_builds() {
151165
let mut rt = spfs_runtime().await;
152-
let remote_repo = spfsrepo().await;
153-
154-
// Populate the "origin" repo with one package.
155-
// The "local" repo is empty.
156-
157-
rt.add_remote_repo(
158-
"origin",
159-
Remote::Address(RemoteAddress {
160-
address: remote_repo.address().clone(),
161-
}),
162-
)
163-
.unwrap();
164-
165-
let recipe = recipe!({"pkg": "my-pkg/1.0.0"});
166-
remote_repo.publish_recipe(&recipe).await.unwrap();
167-
let host_options = HOST_OPTIONS.get().unwrap();
168-
let os_id = host_options.get(OptName::distro()).unwrap();
169-
170-
// Build with all matching host options
171-
let spec = spec!({"pkg": "my-pkg/1.0.0/BGSHW3CN",
172-
"build": {
173-
"options":
174-
[
175-
{"var": format!("{}/{}", OptName::distro(), host_options.get(OptName::distro()).unwrap()) },
176-
{"var": format!("{}/{}", OptName::os(), host_options.get(OptName::os()).unwrap()) },
177-
{"var": format!("{}/{}", OptName::arch(), host_options.get(OptName::arch()).unwrap()) },
178-
{"var": format!("{}/{}", os_id, host_options.get(os_id).unwrap()) }
179-
]
180-
}});
181-
remote_repo
182-
.publish_package(
183-
&spec,
184-
&vec![(Component::Run, empty_layer_digest())]
185-
.into_iter()
186-
.collect(),
187-
)
188-
.await
189-
.unwrap();
190166

191-
// Build with for another distro in its host options
192-
let spec = spec!({"pkg": "my-pkg/1.0.0/2RGMWL2B",
193-
"build": {
194-
"options":
195-
[
196-
{"var": format!("{}/{}", OptName::distro(), "test_distro") },
197-
{"var": format!("{}/{}", OptName::os(), host_options.get(OptName::os()).unwrap()) },
198-
{"var": format!("{}/{}", OptName::arch(), host_options.get(OptName::arch()).unwrap()) },
199-
{"var": format!("{}/{}", os_id, host_options.get(os_id).unwrap()) }
200-
]
201-
}});
202-
remote_repo
203-
.publish_package(
204-
&spec,
205-
&vec![(Component::Run, empty_layer_digest())]
206-
.into_iter()
207-
.collect(),
167+
// Set the host options to make the test deterministic wherever it runs.
168+
// There isn't anything meaningful about the values picked for this.
169+
HOST_OPTIONS.scoped_options(Ok(option_map! {
170+
"os" => "linux",
171+
"arch" => "x86_64",
172+
"distro" => "rocky",
173+
"rocky" => "9.5",
174+
}), async move {
175+
let remote_repo = spfsrepo().await;
176+
177+
// Populate the "origin" repo with one package.
178+
// The "local" repo is empty.
179+
180+
rt.add_remote_repo(
181+
"origin",
182+
Remote::Address(RemoteAddress {
183+
address: remote_repo.address().clone(),
184+
}),
208185
)
209-
.await
210186
.unwrap();
211187

212-
let mut opt = Opt::try_parse_from(["ls", "--host"]).unwrap();
213-
opt.ls.run().await.unwrap();
214-
println!("Output: {:?}", opt.ls.output.vec);
215-
assert_eq!(opt.ls.output.vec.len(), 1);
188+
let recipe = recipe!({"pkg": "my-pkg/1.0.0"});
189+
remote_repo.publish_recipe(&recipe).await.unwrap();
190+
let host_options = HOST_OPTIONS.get().unwrap();
191+
let os_id = host_options.get(OptName::distro()).unwrap();
192+
193+
// Build with all matching host options
194+
let spec = spec!({"pkg": "my-pkg/1.0.0/BGSHW3CN",
195+
"build": {
196+
"options":
197+
[
198+
{"var": format!("{}/{}", OptName::distro(), host_options.get(OptName::distro()).unwrap()) },
199+
{"var": format!("{}/{}", OptName::os(), host_options.get(OptName::os()).unwrap()) },
200+
{"var": format!("{}/{}", OptName::arch(), host_options.get(OptName::arch()).unwrap()) },
201+
{"var": format!("{}/{}", os_id, host_options.get(os_id).unwrap()) }
202+
]
203+
}});
204+
remote_repo
205+
.publish_package(
206+
&spec,
207+
&vec![(Component::Run, empty_layer_digest())]
208+
.into_iter()
209+
.collect(),
210+
)
211+
.await
212+
.unwrap();
213+
214+
// Build with for another distro in its host options
215+
let spec = spec!({"pkg": "my-pkg/1.0.0/2RGMWL2B",
216+
"build": {
217+
"options":
218+
[
219+
{"var": format!("{}/{}", OptName::distro(), "test_distro") },
220+
{"var": format!("{}/{}", OptName::os(), host_options.get(OptName::os()).unwrap()) },
221+
{"var": format!("{}/{}", OptName::arch(), host_options.get(OptName::arch()).unwrap()) },
222+
{"var": format!("{}/{}", os_id, host_options.get(os_id).unwrap()) }
223+
]
224+
}});
225+
remote_repo
226+
.publish_package(
227+
&spec,
228+
&vec![(Component::Run, empty_layer_digest())]
229+
.into_iter()
230+
.collect(),
231+
)
232+
.await
233+
.unwrap();
234+
235+
let mut opt = Opt::try_parse_from(["ls", "--host"]).unwrap();
236+
opt.ls.run().await.unwrap();
237+
println!("Output: {:?}", opt.ls.output.vec);
238+
assert_eq!(opt.ls.output.vec.len(), 1);
239+
240+
Ok::<_, ()>(())
241+
}).await.unwrap();
216242
}
217243

218244
/// `spk ls` is expected to list packages in both the local and the configured

‎crates/spk-schema/crates/foundation/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ ring = { workspace = true }
4242
rstest = { workspace = true }
4343
serde = { workspace = true, features = ["derive"] }
4444
serde_yaml = { workspace = true }
45+
serial_test = { workspace = true }
4546
spfs = { workspace = true }
4647
strum = { workspace = true, features = ["derive"] }
4748
sys-info = "0.9.0"

‎crates/spk-schema/crates/foundation/src/option_map/mod.rs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,12 @@ impl HostOptions {
101101
/// Change [`HOST_OPTIONS`] to return the provided substitute options for
102102
/// the duration of the given future.
103103
///
104-
/// There is no guarantee that some other concurrent task doesn't also
105-
/// change the options before the given future completes.
106-
///
107104
/// This method is intended to only be used by tests.
105+
///
106+
/// There is no guarantee that some other concurrent task doesn't also
107+
/// change the options before the given future completes. It is recommended
108+
/// to use [`serial_test::serial`] and include the key "host_options" when
109+
/// using this function.
108110
pub async fn scoped_options<T>(
109111
&self,
110112
substitute_host_options: Result<OptionMap>,

0 commit comments

Comments
 (0)