Skip to content

Commit

Permalink
- Implement test cases for spec types that has Lints implemented
Browse files Browse the repository at this point in the history
- Fix how lints are outputted to CLI

Signed-off-by: Nichol Yip <[email protected]>
  • Loading branch information
Nichol Yip committed Dec 11, 2024
1 parent 798bba3 commit 4476e05
Show file tree
Hide file tree
Showing 19 changed files with 741 additions and 14 deletions.
12 changes: 10 additions & 2 deletions crates/spk-schema/src/build_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,10 @@ struct BuildSpecVisitor {

impl Lints for BuildSpecVisitor {
fn lints(&mut self) -> Vec<Lint> {
for lint in self.lints.iter_mut() {
lint.update_key("build");
}

std::mem::take(&mut self.lints)
}
}
Expand Down Expand Up @@ -503,11 +507,15 @@ impl<'de> serde::de::Visitor<'de> for BuildSpecVisitor {
"variants" => {
unchecked.raw_variants = map.next_value()?;
}
"validation" => unchecked.validation = map.next_value::<ValidationSpec>()?,
"validation" => {
let linted_validations = map.next_value::<LintedItem<ValidationSpec>>()?;
self.lints.extend(linted_validations.lints);
unchecked.validation = linted_validations.item;
}
"auto_host_vars" => unchecked.auto_host_vars = map.next_value::<AutoHostVars>()?,
unknown_key => {
self.lints.push(Lint::Key(UnknownKey::new(
&format!("build.{unknown_key}"),
&unknown_key,
BuildSpec::FIELD_NAMES_AS_ARRAY.to_vec(),
)));
map.next_value::<serde::de::IgnoredAny>()?;
Expand Down
125 changes: 125 additions & 0 deletions crates/spk-schema/src/build_spec_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use spk_schema_foundation::{option_map, pkg_name, FromYaml, IsDefault};

use super::{AutoHostVars, BuildSpec};
use crate::build_spec::UncheckedBuildSpec;
use crate::LintedItem;

#[rstest]
fn test_auto_host_vars_default() {
Expand Down Expand Up @@ -223,3 +224,127 @@ options:
.unwrap();
assert_ne!(build_id1, build_id2);
}

#[rstest]
fn test_build_script_lint() {
let build_spec: LintedItem<UncheckedBuildSpec> = serde_yaml::from_str(
r#"
options:
- var: arch
- var: os
- var: centos
- pkg: python/3
variants:
- {python: 2.7}
- {python: 3.7, gcc: 9.3}
scripts:
- echo "Hello World!"
"#,
)
.unwrap();

assert_eq!(build_spec.lints.len(), 1);
for lint in build_spec.lints.iter() {
assert_eq!(lint.get_key(), "build.scripts");
}
}

#[rstest]
fn test_build_variant_lint() {
let build_spec: LintedItem<UncheckedBuildSpec> = serde_yaml::from_str(
r#"
options:
- var: arch
- var: os
- var: centos
- pkg: python/3
variant:
- {python: 2.7}
- {python: 3.7, gcc: 9.3}
script:
- echo "Hello World!"
"#,
)
.unwrap();

assert_eq!(build_spec.lints.len(), 1);
for lint in build_spec.lints.iter() {
assert_eq!(lint.get_key(), "build.variant");
}
}

#[rstest]
fn test_build_options_lint() {
let build_spec: LintedItem<UncheckedBuildSpec> = serde_yaml::from_str(
r#"
option:
- var: arch
- var: os
- var: centos
- pkg: python/3
variants:
- {python: 2.7}
- {python: 3.7, gcc: 9.3}
script:
- echo "Hello World!"
"#,
)
.unwrap();

assert_eq!(build_spec.lints.len(), 1);
for lint in build_spec.lints.iter() {
assert_eq!(lint.get_key(), "build.option");
}
}

#[rstest]
fn test_build_auto_host_vars_lint() {
let build_spec: LintedItem<UncheckedBuildSpec> = serde_yaml::from_str(
r#"
options:
- var: arch
- var: os
- var: centos
- pkg: python/3
variants:
- {python: 2.7}
- {python: 3.7, gcc: 9.3}
script:
- echo "Hello World!"
auto_host_var: "None"
"#,
)
.unwrap();

assert_eq!(build_spec.lints.len(), 1);
for lint in build_spec.lints.iter() {
assert_eq!(lint.get_key(), "build.auto_host_var");
}
}

#[rstest]
fn test_build_validation_lint() {
let build_spec: LintedItem<UncheckedBuildSpec> = serde_yaml::from_str(
r#"
options:
- var: arch
- var: os
- var: centos
- pkg: python/3
variants:
- {python: 2.7}
- {python: 3.7, gcc: 9.3}
script:
- echo "Hello World!"
validations: {
"rules": [{"allow": "EmptyPackage"}]
}
"#,
)
.unwrap();

assert_eq!(build_spec.lints.len(), 1);
for lint in build_spec.lints.iter() {
assert_eq!(lint.get_key(), "build.validations");
}
}
4 changes: 4 additions & 0 deletions crates/spk-schema/src/embedded_packages_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ struct EmbeddedPackagesListVisitor {

impl Lints for EmbeddedPackagesListVisitor {
fn lints(&mut self) -> Vec<Lint> {
for lint in self.lints.iter_mut() {
lint.update_key("embedded");
}

std::mem::take(&mut self.lints)
}
}
Expand Down
54 changes: 54 additions & 0 deletions crates/spk-schema/src/embedded_packages_list_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use rstest::rstest;

use super::EmbeddedPackagesList;
use crate::LintedItem;

#[rstest]
fn test_install_embedded_build_options() {
Expand Down Expand Up @@ -45,3 +46,56 @@ fn test_embedded_nested_embedded() {
)
.unwrap();
}

#[rstest]
fn test_embedded_install_lint() {
let _spec: LintedItem<EmbeddedPackagesList> = serde_yaml::from_str(
r#"
- pkg: "embedded/1.0.0"
install:
environments:
- priority: 99
"#,
)
.unwrap();

assert_eq!(_spec.lints.len(), 1);
for lint in _spec.lints.iter() {
assert_eq!(lint.get_key(), "embedded.install.environments")
}
}

#[rstest]
fn test_embedded_components_lint() {
let _spec: LintedItem<EmbeddedPackagesList> = serde_yaml::from_str(
r#"
- pkg: "embedded/1.0.0"
component:
- name: run
embedded:
- pkg: "nested-embedded/2.0.0"
"#,
)
.unwrap();

assert_eq!(_spec.lints.len(), 1);
for lint in _spec.lints.iter() {
assert_eq!(lint.get_key(), "embedded.component")
}
}

#[rstest]
fn test_embedded_build_lint() {
let _spec: LintedItem<EmbeddedPackagesList> = serde_yaml::from_str(
r#"
- pkg: "embedded/1.0.0"
build: {"option": [{"var": "python.abi", "static": "cp37"}]}
"#,
)
.unwrap();

assert_eq!(_spec.lints.len(), 1);
for lint in _spec.lints.iter() {
assert_eq!(lint.get_key(), "embedded.build.option")
}
}
6 changes: 5 additions & 1 deletion crates/spk-schema/src/install_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ where
{
fn lints(&mut self) -> Vec<Lint> {
self.lints.extend(std::mem::take(&mut self.embedded.lints));
for lint in self.lints.iter_mut() {
lint.update_key("install");
}

std::mem::take(&mut self.lints)
}
}
Expand Down Expand Up @@ -260,7 +264,7 @@ where
"environment" => self.environment = map.next_value::<EnvOpList>()?,
unknown_key => {
self.lints.push(Lint::Key(UnknownKey::new(
&format!("install.{unknown_key}"),
&unknown_key,
InstallSpecVisitor::<D>::FIELD_NAMES_AS_ARRAY.to_vec(),
)));
map.next_value::<serde::de::IgnoredAny>()?;
Expand Down
73 changes: 72 additions & 1 deletion crates/spk-schema/src/install_spec_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use spk_schema_foundation::option_map::OptionMap;
use spk_schema_foundation::version::{Version, BINARY_STR};
use spk_schema_ident::{parse_ident_range, PkgRequest, Request, RequestedBy};

use crate::{InstallSpec, Package, RequirementsList};
use crate::{InstallSpec, LintedItem, Package, RawInstallSpec, RequirementsList};

#[rstest]
fn test_render_all_pins_renders_requirements_in_components() {
Expand Down Expand Up @@ -280,3 +280,74 @@ embedded:
"expecting embedded to be expanded correctly"
);
}

#[rstest]
fn test_install_spec_lint_component_field() {
let install = serde_yaml::from_str::<LintedItem<RawInstallSpec>>(
r#"
comzponents:
- name: comp1
"#,
)
.unwrap();

assert_eq!(install.lints.len(), 1);
for lint in install.lints.iter() {
assert_eq!(lint.get_key(), "install.comzponents");
}
}

#[rstest]
fn test_install_spec_lint_embedded_field() {
let install = serde_yaml::from_str::<LintedItem<RawInstallSpec>>(
r#"
components:
- name: comp1
embeddsed:
- pkg: "embedded/1.0.0"
install:
components:
- name: comp1
"#,
)
.unwrap();

assert_eq!(install.lints.len(), 1);
for lint in install.lints.iter() {
assert_eq!(lint.get_key(), "install.embeddsed");
}
}

#[rstest]
fn test_install_spec_lint_requirements_field() {
let install = serde_yaml::from_str::<LintedItem<RawInstallSpec>>(
r#"
requirement:
- pkg: stdfs
"#,
)
.unwrap();

assert_eq!(install.lints.len(), 1);
for lint in install.lints.iter() {
assert_eq!(lint.get_key(), "install.requirement");
}
}

#[rstest]
fn test_install_spec_lint_environments_field() {
let install = serde_yaml::from_str::<LintedItem<RawInstallSpec>>(
r#"
environments:
- priority: 99
- set: PYVER1
value: $SPK_PKG_python_VERSION_BASE
"#,
)
.unwrap();

assert_eq!(install.lints.len(), 1);
for lint in install.lints.iter() {
assert_eq!(lint.get_key(), "install.environments");
}
}
18 changes: 16 additions & 2 deletions crates/spk-schema/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl UnknownKey {

pub fn generate_message(&self) -> String {
let key: &str = self.unknown_key.split('.').last().unwrap_or_default();
let mut message = format!("Unrecognized key: {}. ", self.unknown_key);
let mut message = format!("Unrecognized key: {} ", self.unknown_key);
let mut corpus = CorpusBuilder::new().finish();

for field in self.struct_fields.iter() {
Expand All @@ -31,7 +31,7 @@ impl UnknownKey {

match corpus.search(key, 0.6).first() {
Some(s) => message.push_str(format!("(Did you mean: '{}'?)", s.text).as_str()),
None => message.push_str(format!("(No similar keys found for: {}.)", key).as_str()),
None => message.push_str(format!("(No similar keys found for: {})", key).as_str()),
};

message.to_string()
Expand All @@ -43,6 +43,20 @@ pub enum Lint {
Key(UnknownKey),
}

impl Lint {
pub fn get_key(&self) -> &str {
match self {
Lint::Key(k) => &k.unknown_key,
}
}

pub fn update_key(&mut self, key: &str) {
match self {
Lint::Key(k) => k.unknown_key = format!("{key}.{}", k.unknown_key),
}
}
}

#[derive(Debug, Default, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
pub struct LintedItem<T> {
pub item: T,
Expand Down
Loading

0 comments on commit 4476e05

Please sign in to comment.