Let's have exhaustive feature lists!

[ljharb]

Opening this per https://github.com/tc39/test262/pull/3353#issuecomment-1247158634.

I think that in order to work towards userland packages being able to run test262 themselves, we need to do a better job ensuring that feature lists are exhaustive, and that unnecessary features aren't used (arrow functions when normal ones suffice, for example), without putting a burden on contributors.

I don't think a precommit script is a good idea specifically - either we need a way for maintainers to easily clean things up later, or, we need a CI check/formatter that helps contributors do the right thing without having to type it all out.

[ptomato]

Personally I'd like to see this be treated as opt-in; I'm thinking along the lines of contributors should not have to pay attention to features that have been stage 4 for a while, so we shouldn't do this in CI, nor necessarily use a script to always automatically transform all arrow functions into normal functions, for example. But we should provide scripts to make it easy for userland packages to adjust whatever features they need on whatever subset of tests they need to do it on, and generally accept such PRs from userland packages without complaint.

In other words, a best-effort supported use case where that we facilitate where possible while not requiring additional effort from constituents higher on our list.

[ljharb]

I'm totally fine with avoiding burden!

I'm not sure how practical it is tho for consumers to run scripts to mutate test262 on disk in order to run it.

[ptomato]

I agree, I'm saying we should be willing to commit changes like this without necessarily making it a priority to make them preemptively.

[ljharb]

I agree it doesn’t need to block merging, especially if we have a manual tool to detect and/or autofix them later.

[anba]

Past discussions which are probably relevant here: #3196, #2862, #472 (links to this meeting note), #569. I think there are more, for example in review comments when the reviewer encouraged to use const instead of var or other other way around, i.e. to use var instead of const to target older engines.

[ptomato]

Related to this, it also occurred to me that for features that long ago reached Stage 4, we could organize the features list according to what platforms are likely to want to run the tests. For example, we could remove all the individual typed array feature flags (Uint8Array, Int8Array, ...), and just use TypedArray as a catch-all feature flag, if there aren't any platforms where we want to run the tests that only implement a subset of the typed arrays.

[ljharb]

Simplifying the feature flags sounds great - but personally I'd want to eventually run the tests on a ton of older browser/engine versions, so for typed arrays i'd want to confirm that there aren't any engines that implemented the pre-bigint TAs piecemeal.

[p-bakker]

since this is about the features list, I'm asking here, but if I should be asking anywhere else, please let me know

I'm trying to determine how to interpret the features.txt file. I naively assumes that all tests are properly annotated with the features they test/use for testing something else, so I wrote a script that runs all the test against Rhino and see for which features we have no passing tests at all, which should indicate that those are features Rhino indeed doesn't support (so I semi-automatically keep the list of features we don't support up to date by detecting new features), as we use that list to skip tests that would fail anyway

However, got a lot of false positives

So am looking for guidance how to interpret features.txt: inside the file it says To filter for specific APIs, use the directory structure, but I cannot really directly correlate the feature string with a (full) directory name, for example:

• FinalizationRegistry.prototype.cleanupSome
• Intl.NumberFormat-v3
• legacy-regexp
• Atomics.waitAsync

Any insight appreciated

[ptomato]

Yes, I'm afraid some features aren't annotated particularly consistently right now.

I think what "To filter for specific APIs, use the directory structure" means is that if you don't support Atomics.waitAsync, then skip test/{built-ins,intl402,staging}/Atomics/waitAsync/**. For features that aren't an API surface, like legacy-regexp, I agree that's not very helpful.