Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse non-true values for disabled properties #143

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions moz-webgpu-cts/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ publish = false
dist = true

[dependencies]
arcstr = { version = "1.1.5", features = ["serde"] }
camino = { version = "1.1.6", features = ["serde1"] }
clap = { version = "4.4.2", features = ["derive"] }
env_logger = "0.10.0"
Expand Down
8 changes: 4 additions & 4 deletions moz-webgpu-cts/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1046,14 +1046,14 @@ fn run(cli: Cli) -> ExitCode {
} = test;

let TestProps {
is_disabled,
disabled,
expected,
implementation_status: _,
} = properties;

let test_name = Arc::new(test_name);

if is_disabled {
if disabled.is_some() {
analysis.for_each_platform_mut(|analysis| {
analysis
.tests_with_disabled_or_skip
Expand Down Expand Up @@ -1161,12 +1161,12 @@ fn run(cli: Cli) -> ExitCode {

let Subtest { properties } = subtest;
let TestProps {
is_disabled,
disabled,
expected,
implementation_status: _,
} = properties;

if is_disabled {
if disabled.is_some() {
analysis
.windows
.tests_with_disabled_or_skip
Expand Down
72 changes: 33 additions & 39 deletions moz-webgpu-cts/src/wpt/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::{
hash::Hash,
};

use arcstr::ArcStr;
use clap::ValueEnum;
use enum_map::Enum;
use enumset::EnumSetType;
Expand Down Expand Up @@ -31,7 +32,9 @@ use whippit::{
},
};

use crate::wpt::metadata::properties::{ExpandedPropertyValue, Expected, NormalizedPropertyValue};
use crate::wpt::metadata::properties::{
DisabledString, ExpandedPropertyValue, Expected, NormalizedPropertyValue,
};

#[cfg(test)]
use insta::assert_debug_snapshot;
Expand Down Expand Up @@ -63,7 +66,7 @@ impl<'a> metadata::File<'a> for File {

#[derive(Clone, Debug, Default, Serialize)]
pub struct FileProps {
pub is_disabled: Option<PropertyValue<Expr<Value<'static>>, String>>,
pub disabled: Option<PropertyValue<Expr<Value<'static>>, DisabledString>>,
#[allow(clippy::type_complexity)]
pub prefs: Option<PropertyValue<Expr<Value<'static>>, Vec<(String, String)>>>,
pub tags: Option<PropertyValue<Expr<Value<'static>>, Vec<String>>>,
Expand Down Expand Up @@ -139,11 +142,11 @@ impl<'a> Properties<'a> for FileProps {
any()
.and_is(newline().or(end()).not())
.repeated()
.at_least(1)
.to_slice()
.map(|s: &str| s.to_owned()),
.map(ArcStr::from)
.map(DisabledString::new),
)
.map(|((), is_disabled)| FileProp::Disabled(is_disabled));
.map(|((), val)| FileProp::Disabled(val));

let implementation_status = helper
.parser(
Expand All @@ -164,7 +167,7 @@ impl<'a> Properties<'a> for FileProps {
let (span, prop) = prop;
let Self {
implementation_status,
is_disabled,
disabled,
prefs,
tags,
} = self;
Expand All @@ -190,7 +193,7 @@ impl<'a> Properties<'a> for FileProps {
FileProp::Prefs(new_prefs) => check_dupe_then_insert!(new_prefs, prefs, "prefs"),
FileProp::Tags(new_tags) => check_dupe_then_insert!(new_tags, tags, "tags"),
FileProp::Disabled(new_is_disabled) => {
check_dupe_then_insert!(new_is_disabled, is_disabled, DISABLED_IDENT)
check_dupe_then_insert!(new_is_disabled, disabled, DISABLED_IDENT)
}
}
}
Expand Down Expand Up @@ -453,7 +456,7 @@ const DISABLED_IDENT: &str = "disabled";
pub enum FileProp {
Prefs(PropertyValue<Expr<Value<'static>>, Vec<(String, String)>>),
Tags(PropertyValue<Expr<Value<'static>>, Vec<String>>),
Disabled(PropertyValue<Expr<Value<'static>>, String>),
Disabled(PropertyValue<Expr<Value<'static>>, DisabledString>),
ImplementationStatus(PropertyValue<Expr<Value<'static>>, ImplementationStatus>),
}

Expand Down Expand Up @@ -521,7 +524,7 @@ fn format_file_properties(props: &FileProps) -> impl Display + '_ {
lazy_format!(|f| {
let FileProps {
implementation_status,
is_disabled,
disabled,
prefs,
tags,
} = props;
Expand Down Expand Up @@ -559,8 +562,8 @@ fn format_file_properties(props: &FileProps) -> impl Display + '_ {
)?;
}

if let Some(is_disabled) = is_disabled {
write_prop_val(DISABLED_IDENT, is_disabled, Display::fmt, f)?;
if let Some(disabled) = disabled {
write_prop_val(DISABLED_IDENT, disabled, Display::fmt, f)?;
}

Ok(())
Expand Down Expand Up @@ -749,15 +752,11 @@ where
.join_with(" ")
));
let TestProps {
is_disabled,
disabled,
expected,
implementation_status,
} = property;

if *is_disabled {
writeln!(f, "{indent}disabled: true")?;
}

fn write_normalized<T>(
f: &mut Formatter<'_>,
indent: &dyn Display,
Expand Down Expand Up @@ -841,6 +840,10 @@ where
)?;
}

if let Some(disabled) = disabled {
write_normalized(f, &indent, "disabled", disabled)?;
}

if let Some(exps) = expected {
write_normalized(f, &indent, EXPECTED_IDENT, exps)?;
}
Expand All @@ -867,7 +870,7 @@ pub struct TestProps<Out>
where
Out: EnumSetType,
{
pub is_disabled: bool,
pub disabled: Option<ExpandedPropertyValue<DisabledString>>,
pub expected: Option<ExpandedPropertyValue<Expected<Out>>>,
pub implementation_status: Option<ExpandedPropertyValue<ImplementationStatus>>,
}
Expand All @@ -878,7 +881,7 @@ where
{
fn insert(&mut self, prop: TestProp<Out>, emitter: &mut Emitter<Rich<'a, char>>) {
let Self {
is_disabled,
disabled,
expected,
implementation_status,
} = self;
Expand Down Expand Up @@ -946,11 +949,8 @@ where
TestPropKind::Expected(val) => {
conditional(emitter, span, EXPECTED_IDENT, expected, val)
}
TestPropKind::Disabled => {
if *is_disabled {
emitter.emit(Rich::custom(span, "duplicate `disabled` key detected"))
}
*is_disabled = true;
TestPropKind::Disabled(val) => {
conditional(emitter, span, DISABLED_IDENT, disabled, val)
}
TestPropKind::ImplementationStatus(val) => conditional(
emitter,
Expand Down Expand Up @@ -984,7 +984,7 @@ where
Out: EnumSetType,
{
Expected(PropertyValue<Applicability, Expected<Out>>),
Disabled,
Disabled(PropertyValue<Applicability, DisabledString>),
ImplementationStatus(PropertyValue<Applicability, ImplementationStatus>),
}

Expand Down Expand Up @@ -1143,22 +1143,16 @@ where
.parser(
just(DISABLED_IDENT).to(()),
conditional_term.clone(),
just("true").to(()),
any()
.and_is(newline().not())
.repeated()
.to_slice()
.map(ArcStr::from)
.map(DisabledString::new),
)
.validate(|((), val), e, emitter| {
match val {
PropertyValue::Unconditional(()) => (),
PropertyValue::Conditional { .. } => {
emitter.emit(Rich::custom(
e.span(),
"conditional rules for `disabled` aren't supported yet",
));
}
}
TestProp {
span: e.span(),
kind: TestPropKind::Disabled,
}
.map_with(|((), val), e| TestProp {
span: e.span(),
kind: TestPropKind::Disabled(val),
}),
helper
.parser(
Expand Down
25 changes: 25 additions & 0 deletions moz-webgpu-cts/src/wpt/metadata/properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
ops::{BitOr, BitOrAssign, Index, IndexMut},
};

use arcstr::ArcStr;
use enum_map::EnumMap;
use enumset::{EnumSet, EnumSetType};
use itertools::Itertools;
Expand Down Expand Up @@ -206,6 +207,30 @@

impl<Out> Eq for Expected<Out> where Out: EnumSetType + Eq {}

#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize)]
pub struct DisabledString(ArcStr);

impl DisabledString {
const FALSE: ArcStr = arcstr::literal!("@False");

Check failure on line 214 in moz-webgpu-cts/src/wpt/metadata/properties.rs

View workflow job for this annotation

GitHub Actions / Clippy (beta)

a `const` item should not be interior mutable

Check failure on line 214 in moz-webgpu-cts/src/wpt/metadata/properties.rs

View workflow job for this annotation

GitHub Actions / Clippy (beta)

a `const` item should not be interior mutable

pub fn new(inner: ArcStr) -> Self {
Self(if inner == Self::FALSE {

Check failure on line 217 in moz-webgpu-cts/src/wpt/metadata/properties.rs

View workflow job for this annotation

GitHub Actions / Clippy (beta)

a `const` item with interior mutability should not be borrowed
Self::FALSE
} else {
inner
})
}

pub fn value(&self) -> &str {
self.0.as_ref()
}
}

impl Display for DisabledString {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
Display::fmt(self.value(), f)
}
}
/// A completely flat representation of [`NormalizedPropertyValue`] suitable for byte
/// representation in memory.
#[derive(Debug, Default, Clone, Copy, Eq, PartialEq, Serialize)]
Expand Down
Loading