From a0b592a8ca8dd9a2f16a022762275f4b47bd43f8 Mon Sep 17 00:00:00 2001 From: "Ruben Arts(win)" Date: Mon, 6 May 2024 20:56:12 +0200 Subject: [PATCH 1/3] test: add test --- src/project/environment.rs | 57 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/project/environment.rs b/src/project/environment.rs index cfceca09a..abf2fb47b 100644 --- a/src/project/environment.rs +++ b/src/project/environment.rs @@ -490,6 +490,63 @@ mod tests { ); } + fn test_channel_feature_priority(){ + let manifest = Project::from_str( + Path::new("pixi.toml"), + r#" + [project] + name = "foobar" + channels = ["a", "b"] + platforms = ["linux-64", "osx-64"] + + [feature.foo] + channels = ["c", "d"] + + [feature.bar] + channels = ["e", "f"] + + [feature.barfoo] + channels = ["a", "f"] + + [environments] + foo = ["foo"] + bar = ["bar"] + foobar = ["foo", "bar"] + barfoo = {features = ["barfoo"], no-default-features=true} + "#, + ) + .unwrap(); + + let foobar_channels = manifest.environment("foobar").unwrap().channels(); + assert_eq!( + foobar_channels + .into_iter() + .map(|c| c.name.clone().unwrap()) + .collect_vec(), + vec!["a", "b", "c", "d", "e", "f"] + ); + let foo_channels = manifest.environment("foo").unwrap().channels(); + assert_eq!( + foo_channels + .into_iter() + .map(|c| c.name.clone().unwrap()) + .collect_vec(), + vec!["a", "b", "c", "d"] + ); + + let bar_channels = manifest.environment("bar").unwrap().channels(); + assert_eq!( + bar_channels + .into_iter() + .map(|c| c.name.clone().unwrap()) + .collect_vec(), + vec!["a", "b", "e", "f"] + ); + + let barfoo_channels = manifest.environment("barfoo").unwrap().channels(); + assert_eq!(barfoo_channels.into_iter().map(|c| c.name.clone().unwrap()).collect_vec(), vec!["a", "f"]) + } + #[test] fn test_channel_priorities() { let manifest = Project::from_str( From bf190bab30adbd596a4bbdcde2a14f135224760b Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Mon, 6 May 2024 21:21:48 +0200 Subject: [PATCH 2/3] wip: fix tests --- src/project/environment.rs | 58 ++++++++++++++++++++++++++++++++----- src/project/has_features.rs | 6 ++-- 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/src/project/environment.rs b/src/project/environment.rs index abf2fb47b..f2e7406c8 100644 --- a/src/project/environment.rs +++ b/src/project/environment.rs @@ -490,7 +490,8 @@ mod tests { ); } - fn test_channel_feature_priority(){ + #[test] + fn test_channel_feature_priority() { let manifest = Project::from_str( Path::new("pixi.toml"), r#" @@ -512,10 +513,10 @@ mod tests { foo = ["foo"] bar = ["bar"] foobar = ["foo", "bar"] - barfoo = {features = ["barfoo"], no-default-features=true} + barfoo = {features = ["barfoo"], no-default-feature=true} "#, ) - .unwrap(); + .unwrap(); let foobar_channels = manifest.environment("foobar").unwrap().channels(); assert_eq!( @@ -523,7 +524,7 @@ mod tests { .into_iter() .map(|c| c.name.clone().unwrap()) .collect_vec(), - vec!["a", "b", "c", "d", "e", "f"] + vec!["c", "d", "e", "f", "a", "b"] ); let foo_channels = manifest.environment("foo").unwrap().channels(); assert_eq!( @@ -531,7 +532,7 @@ mod tests { .into_iter() .map(|c| c.name.clone().unwrap()) .collect_vec(), - vec!["a", "b", "c", "d"] + vec!["c", "d", "a", "b"] ); let bar_channels = manifest.environment("bar").unwrap().channels(); @@ -540,11 +541,54 @@ mod tests { .into_iter() .map(|c| c.name.clone().unwrap()) .collect_vec(), - vec!["a", "b", "e", "f"] + vec!["e", "f", "a", "b"] ); let barfoo_channels = manifest.environment("barfoo").unwrap().channels(); - assert_eq!(barfoo_channels.into_iter().map(|c| c.name.clone().unwrap()).collect_vec(), vec!["a", "f"]) + assert_eq!( + barfoo_channels + .into_iter() + .map(|c| c.name.clone().unwrap()) + .collect_vec(), + vec!["a", "f"] + ) + } + + #[test] + fn test_channel_feature_priority_with_redifinition() { + let manifest = Project::from_str( + Path::new("pixi.toml"), + r#" + [project] + name = "holoviews" + channels = ["a", "b"] + platforms = ["linux-64"] + + [environments] + foo = ["foo"] + + [feature.foo] + channels = ["a", "c", "b"] + "#, + ) + .unwrap(); + + let foobar_channels = manifest.environment("default").unwrap().channels(); + assert_eq!( + foobar_channels + .into_iter() + .map(|c| c.name.clone().unwrap()) + .collect_vec(), + vec!["a", "b"] + ); + let foo_channels = manifest.environment("foo").unwrap().channels(); + assert_eq!( + foo_channels + .into_iter() + .map(|c| c.name.clone().unwrap()) + .collect_vec(), + vec!["a", "c", "b"] + ); } #[test] diff --git a/src/project/has_features.rs b/src/project/has_features.rs index 49bfccb5f..361a8db34 100644 --- a/src/project/has_features.rs +++ b/src/project/has_features.rs @@ -27,15 +27,15 @@ pub trait HasFeatures<'p> { fn channels(&self) -> IndexSet<&'p Channel> { // We reverse once before collecting into an IndexSet, and once after, // to ensure the default channels of the project are added to the end of the list. - let mut channels: IndexSet<_> = self + let channels: IndexSet<_> = self .features() .flat_map(|feature| match &feature.channels { Some(channels) => channels, None => &self.project().manifest.parsed.project.channels, }) - .rev() + // .rev() .collect(); - channels.reverse(); + // channels.reverse(); // The prioritized channels contain a priority, sort on this priority. // Higher priority comes first. [-10, 1, 0 ,2] -> [2, 1, 0, -10] From 9fad4114f2b7048963a0db4093ce9649b9b6e58d Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Mon, 6 May 2024 21:59:34 +0200 Subject: [PATCH 3/3] fix: cleanup and complete the tests. --- src/project/environment.rs | 29 ++++++++++++----------------- src/project/has_features.rs | 6 ++---- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/src/project/environment.rs b/src/project/environment.rs index f2e7406c8..b00e77dbe 100644 --- a/src/project/environment.rs +++ b/src/project/environment.rs @@ -321,8 +321,6 @@ mod tests { ); } - // TODO: Add a test to verify that feature specific channels work as expected. - #[test] fn test_default_platforms() { let manifest = Project::from_str( @@ -511,13 +509,13 @@ mod tests { [environments] foo = ["foo"] - bar = ["bar"] foobar = ["foo", "bar"] barfoo = {features = ["barfoo"], no-default-feature=true} "#, ) .unwrap(); + // All channels are added in order of the features and default is last let foobar_channels = manifest.environment("foobar").unwrap().channels(); assert_eq!( foobar_channels @@ -526,6 +524,7 @@ mod tests { .collect_vec(), vec!["c", "d", "e", "f", "a", "b"] ); + let foo_channels = manifest.environment("foo").unwrap().channels(); assert_eq!( foo_channels @@ -535,15 +534,7 @@ mod tests { vec!["c", "d", "a", "b"] ); - let bar_channels = manifest.environment("bar").unwrap().channels(); - assert_eq!( - bar_channels - .into_iter() - .map(|c| c.name.clone().unwrap()) - .collect_vec(), - vec!["e", "f", "a", "b"] - ); - + // The default feature is not included in the channels, so only the feature channels are included. let barfoo_channels = manifest.environment("barfoo").unwrap().channels(); assert_eq!( barfoo_channels @@ -555,13 +546,13 @@ mod tests { } #[test] - fn test_channel_feature_priority_with_redifinition() { + fn test_channel_feature_priority_with_redefinition() { let manifest = Project::from_str( Path::new("pixi.toml"), r#" [project] - name = "holoviews" - channels = ["a", "b"] + name = "test" + channels = ["d", "a", "b"] platforms = ["linux-64"] [environments] @@ -569,6 +560,7 @@ mod tests { [feature.foo] channels = ["a", "c", "b"] + "#, ) .unwrap(); @@ -579,15 +571,18 @@ mod tests { .into_iter() .map(|c| c.name.clone().unwrap()) .collect_vec(), - vec!["a", "b"] + vec!["d", "a", "b"] ); + + // Check if the feature channels are sorted correctly, + // and that the remaining channels from the default feature are appended. let foo_channels = manifest.environment("foo").unwrap().channels(); assert_eq!( foo_channels .into_iter() .map(|c| c.name.clone().unwrap()) .collect_vec(), - vec!["a", "c", "b"] + vec!["a", "c", "b", "d"] ); } diff --git a/src/project/has_features.rs b/src/project/has_features.rs index 361a8db34..6c133c6a9 100644 --- a/src/project/has_features.rs +++ b/src/project/has_features.rs @@ -25,17 +25,15 @@ pub trait HasFeatures<'p> { /// If a feature does not specify any channel the default channels from the project metadata are /// used instead. fn channels(&self) -> IndexSet<&'p Channel> { - // We reverse once before collecting into an IndexSet, and once after, - // to ensure the default channels of the project are added to the end of the list. + // Collect all the channels from the features in one set, + // deduplicate them and sort them on feature index, default feature comes last. let channels: IndexSet<_> = self .features() .flat_map(|feature| match &feature.channels { Some(channels) => channels, None => &self.project().manifest.parsed.project.channels, }) - // .rev() .collect(); - // channels.reverse(); // The prioritized channels contain a priority, sort on this priority. // Higher priority comes first. [-10, 1, 0 ,2] -> [2, 1, 0, -10]