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

Document implementation-contributed #4105

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dminor
Copy link
Contributor

@dminor dminor commented Jun 3, 2024

This is based upon the staging section. Unlike the previous implementation-contributed, this one is a subdirectory under tests.

I've left the requirement for the tests to be runnable under the usual test262 runner. This isn't a problem for SpiderMonkey, since we have a conversion script, but we could drop it if we think this is too much of a burden for other implementations. Personally, I think implementation-contributed will be more useful if we keep it.

@dminor dminor requested a review from a team as a code owner June 3, 2024 17:31
@ljharb
Copy link
Member

ljharb commented Jun 3, 2024

I'm a bit confused - what's the benefit of this over just using staging?

@dminor
Copy link
Contributor Author

dminor commented Jun 3, 2024

I'm a bit confused - what's the benefit of this over just using staging?

The staging name implies something temporary, as does the current documentation in CONTRIBUTING.md. Personally, I don't have a strong opinion, we could just modify the documentation for staging to say it's ok for things to stay there indefinitely.

@ljharb
Copy link
Member

ljharb commented Jun 3, 2024

It is temporary - the ideal state is for everything in staging to have been moved into the full test suite.

@dminor
Copy link
Contributor Author

dminor commented Jun 3, 2024

In that case, I think implementation-contributed is the right place. The idea is for implementations to share existing tests that won't eventually become normal test262 tests.

@ljharb
Copy link
Member

ljharb commented Jun 3, 2024

What sort of tests do you imagine more than one implementation would want to run, that all implementations wouldn't want to run?

@dminor
Copy link
Contributor Author

dminor commented Jun 3, 2024

We have thousands of existing tests that would benefit other implementations, that we will never, ever have the time to rewrite as proper test262 tests. By upstreaming mechanically converted versions, we'd make them more discoverable and usable by other implementers.

Let me turn the question around, what is the downside that you see of us upstreaming these tests?

@ljharb
Copy link
Member

ljharb commented Jun 3, 2024

I don't see a downside of upstreaming tests that would benefit other implementations - that's great! However, whether "temporary" lasts weeks or decades, it still seems better if they're eventually migrated - by somebody - to the main suite.

@Ms2ger
Copy link
Contributor

Ms2ger commented Jun 4, 2024

Sure. The question is: does adding tests to staging imply a commitment from the person adding them to spend time on that? If not, perhaps staging works, but we might want to clarify the documentation

@dminor
Copy link
Contributor Author

dminor commented Jun 4, 2024

I think from our point of view what we would want is a promise that tests won't be removed from staging unless there's a regular test that covers the same things.

@ljharb
Copy link
Member

ljharb commented Jun 4, 2024

Sure. The question is: does adding tests to staging imply a commitment from the person adding them to spend time on that? If not, perhaps staging works, but we might want to clarify the documentation

No, it doesn't - it's on us, the test262 maintainers, to do it eventually if nobody else does.

I think from our point of view what we would want is a promise that tests won't be removed from staging unless there's a regular test that covers the same things.

That is already the way staging works :-)

@ptomato
Copy link
Contributor

ptomato commented Jul 24, 2024

We really need to clear up the disagreement and move this forward. I'll add it to the next meeting agenda.

@ptomato
Copy link
Contributor

ptomato commented Jul 25, 2024

Speaking for myself, I think implementation-contributed is fine for these. I'd like to put automatically migrated tests in i-c, for which the source of truth is upstream in the implementation. For me, the migration path would be to move the source of truth to test262, possibly deleting the upstream tests. Converting i-c tests to "proper" tests and deleting them from i-c would make a huge mess if we wanted to continue to run the automatic migration periodically.

I see staging as similar but different. The source of truth for staging is in test262. There, the migration path is converting the tests to "proper" tests and deleting them from staging.

(in other words, if the tests are being converted once with a script, landed here, and then deleted from Mozilla's codebase, then sure, staging is better; otherwise, i-c)

I don't think I'd want the test262 maintainers to accept the responsibility of migrating tests either from i-c or staging just for the sake of it; that seems like really low-ROI work to me unless there is a reason (like getting to the required coverage for stage 3, which neither staging nor i-c count towards!)

Anyway, that's my current thinking, I'm open to being convinced otherwise. I've put it on the agenda for the next maintainers meeting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants