Skip to content

fix(tester): correct edition mapping for ES16 #5146

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.

Sign up for GitHub

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

Merged
jedel1043 merged 3 commits into from
Mar 19, 2026
Merged

fix(tester): correct edition mapping for ES16 #5146

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4d29e38
fix(tester): correct edition mapping for ES16 (#5053)
Xavrir Mar 19, 2026
e4a5a7e
test: cover ES16 tester edition plumbing
Xavrir Mar 19, 2026
50e871f
test: drop brittle ES16 tester coverage
Xavrir Mar 19, 2026
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
62 changes: 57 additions & 5 deletions tests/tester/src/edition.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,6 @@ static FEATURE_EDITION: phf::Map<&'static str, SpecEdition> = phf::phf_map! {
// https://github.com/tc39/proposal-json-modules
"json-modules" => SpecEdition::ESNext,

// ArrayBuffer transfer
// https://github.com/tc39/proposal-arraybuffer-transfer
"arraybuffer-transfer" => SpecEdition::ESNext,

// Intl.DurationFormat
// https://github.com/tc39/proposal-intl-duration-format
"Intl.DurationFormat" => SpecEdition::ESNext,
Expand Down Expand Up @@ -158,6 +154,8 @@ static FEATURE_EDITION: phf::Map<&'static str, SpecEdition> = phf::phf_map! {
"Atomics.waitAsync" => SpecEdition::ES15,
"align-detached-buffer-semantics-with-web-reality" => SpecEdition::ES12,
"arbitrary-module-namespace-names" => SpecEdition::ES13,
// https://github.com/tc39/proposal-arraybuffer-transfer
"arraybuffer-transfer" => SpecEdition::ES15,
"array-grouping" => SpecEdition::ES15,
"ArrayBuffer" => SpecEdition::ES6,
"array-find-from-last" => SpecEdition::ES14,
Expand Down Expand Up @@ -277,7 +275,8 @@ static FEATURE_EDITION: phf::Map<&'static str, SpecEdition> = phf::phf_map! {
"String.prototype.toWellFormed" => SpecEdition::ES15,
"String.prototype.trimEnd" => SpecEdition::ES10,
"String.prototype.trimStart" => SpecEdition::ES10,
"set-methods" => SpecEdition::ES15,
// https://github.com/tc39/proposal-set-methods
"set-methods" => SpecEdition::ES16,
"super" => SpecEdition::ES6,
"Symbol" => SpecEdition::ES6,
"symbols-as-weakmap-keys" => SpecEdition::ES14,
Expand Down Expand Up @@ -379,6 +378,10 @@ pub(crate) enum SpecEdition {
///
/// <https://262.ecma-international.org/15.0>
ES15,
/// ECMAScript 16th Edition
///
/// <https://262.ecma-international.org/16.0>
ES16,
/// The edition being worked on right now.
///
/// A draft is currently available [here](https://tc39.es/ecma262).
Expand Down Expand Up @@ -443,8 +446,57 @@ impl SpecEdition {
Self::ES13,
Self::ES14,
Self::ES15,
Self::ES16,
Self::ESNext,
]
.into_iter()
}
}

#[cfg(test)]
mod tests {
use super::SpecEdition;
use crate::{
Locale,
read::{MetaData, TestFlag},
};

fn metadata(features: &[&str]) -> MetaData {
MetaData {
description: "test".into(),
esid: None,
es5id: None,
es6id: None,
info: "".into(),
features: features
.iter()
.map(|feature| (*feature).into())
.collect::<Vec<Box<str>>>()
.into_boxed_slice(),
includes: Box::default(),
flags: Box::<[TestFlag]>::default(),
negative: None,
locale: Locale::default(),
}
}

#[test]
fn maps_arraybuffer_transfer_to_es15() {
let metadata = metadata(&["arraybuffer-transfer"]);

assert_eq!(
SpecEdition::from_test_metadata(&metadata),
Ok(SpecEdition::ES15)
);
}

#[test]
fn maps_set_methods_to_es16() {
let metadata = metadata(&["set-methods"]);

assert_eq!(
SpecEdition::from_test_metadata(&metadata),
Ok(SpecEdition::ES16)
);
}
}
Copy link
Member

@jedel1043 jedel1043 Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need tests for these. Not because tests are bad or anything, but because the editions mapping is so in-flight sometimes that it's not really useful to test if specific features map to a certain version.

Case in point, if we had tests for these, then you wouldn't have been able to change the edition of these two features without changing the tests.

Copy link
Contributor Author

@Xavrir Xavrir Mar 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, I removed those mapping tests in 50e871f. You’re right that pinning exact feature editions there would just create churn.

69 changes: 69 additions & 0 deletions tests/tester/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,7 @@ struct VersionedStats {
es13: Statistics,
es14: Statistics,
es15: Statistics,
es16: Statistics,
}

impl<'de> Deserialize<'de> for VersionedStats {
Expand All @@ -622,6 +623,8 @@ impl<'de> Deserialize<'de> for VersionedStats {
es14: Option<Statistics>,
#[serde(default)]
es15: Option<Statistics>,
#[serde(default)]
es16: Option<Statistics>,
}

let inner = Inner::deserialize(deserializer)?;
Expand All @@ -638,9 +641,11 @@ impl<'de> Deserialize<'de> for VersionedStats {
es13,
es14,
es15,
es16,
} = inner;
let es14 = es14.unwrap_or(es13);
let es15 = es15.unwrap_or(es14);
let es16 = es16.unwrap_or(es15);

Ok(Self {
es5,
Expand All @@ -654,6 +659,7 @@ impl<'de> Deserialize<'de> for VersionedStats {
es13,
es14,
es15,
es16,
})
}
}
Expand Down Expand Up @@ -684,6 +690,7 @@ impl VersionedStats {
SpecEdition::ES13 => self.es13,
SpecEdition::ES14 => self.es14,
SpecEdition::ES15 => self.es15,
SpecEdition::ES16 => self.es16,
SpecEdition::ESNext => return None,
};
Some(stats)
Expand All @@ -704,6 +711,7 @@ impl VersionedStats {
SpecEdition::ES13 => &mut self.es13,
SpecEdition::ES14 => &mut self.es14,
SpecEdition::ES15 => &mut self.es15,
SpecEdition::ES16 => &mut self.es16,
SpecEdition::ESNext => return None,
};
Some(stats)
Expand All @@ -726,6 +734,7 @@ impl Add for VersionedStats {
es13: self.es13 + rhs.es13,
es14: self.es14 + rhs.es14,
es15: self.es15 + rhs.es15,
es16: self.es16 + rhs.es16,
}
}
}
Expand All @@ -743,6 +752,66 @@ impl AddAssign for VersionedStats {
self.es13 += rhs.es13;
self.es14 += rhs.es14;
self.es15 += rhs.es15;
self.es16 += rhs.es16;
}
}

#[cfg(test)]
mod tests {
use super::{SpecEdition, Statistics, VersionedStats};

fn mark_passed(entry: &mut Statistics) {
entry.total += 1;
entry.passed += 1;
}

#[test]
fn deserializes_es16_from_older_results() {
let stats: VersionedStats = serde_json::from_value(serde_json::json!({
"es5": {"t": 1, "o": 1, "i": 0, "p": 0},
"es6": {"t": 2, "o": 2, "i": 0, "p": 0},
"es7": {"t": 3, "o": 3, "i": 0, "p": 0},
"es8": {"t": 4, "o": 4, "i": 0, "p": 0},
"es9": {"t": 5, "o": 5, "i": 0, "p": 0},
"es10": {"t": 6, "o": 6, "i": 0, "p": 0},
"es11": {"t": 7, "o": 7, "i": 0, "p": 0},
"es12": {"t": 8, "o": 8, "i": 0, "p": 0},
"es13": {"t": 9, "o": 9, "i": 0, "p": 0},
"es14": {"t": 10, "o": 10, "i": 0, "p": 0},
"es15": {"t": 11, "o": 11, "i": 0, "p": 0}
}))
.expect("older result schema should deserialize");

let es15 = stats
.get(SpecEdition::ES15)
.expect("ES15 stats should exist");
let es16 = stats
.get(SpecEdition::ES16)
.expect("ES16 stats should exist");

assert_eq!(es16.total, es15.total);
assert_eq!(es16.passed, es15.passed);
assert_eq!(es16.ignored, es15.ignored);
assert_eq!(es16.panic, es15.panic);
}

#[test]
fn apply_updates_es16_statistics() {
Copy link
Member

@jedel1043 jedel1043 Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test doesn't really reflect how we keep count of tests; usually if ES6 has a test, that test also counts for the total in ES7 and onwards, so it doesn't make sense to have ES15 with a lower test count than ES16

Copy link
Contributor Author

@Xavrir Xavrir Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the extra ES16 stats tests in 50e871f and kept the PR focused on the edition fix plus the required tester plumbing.

let mut stats = VersionedStats::default();

stats.apply(SpecEdition::ES16, mark_passed);

let es15 = stats
.get(SpecEdition::ES15)
.expect("ES15 stats should exist");
let es16 = stats
.get(SpecEdition::ES16)
.expect("ES16 stats should exist");

assert_eq!(es15.total, 0);
assert_eq!(es15.passed, 0);
assert_eq!(es16.total, 1);
assert_eq!(es16.passed, 1);
}
}

Expand Down
Loading