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

Language revision dependent spectests: change the approach. #31

Closed
vrurg opened this issue May 23, 2019 · 18 comments
Closed

Language revision dependent spectests: change the approach. #31

vrurg opened this issue May 23, 2019 · 18 comments
Assignees
Labels
language Changes to the Raku Programming Language

Comments

@vrurg
Copy link
Contributor

vrurg commented May 23, 2019

Preamble

Correct me if I'm wrong, but from docs and from the code it looks like currently each language revision needs its own individual spectest.data.6.<rev> which would largely duplicate the default spectest.data file. Besides, what is tested is dependent on the content of the VERSION file which belongs to roast and somewhat bound to Rakudo versioning. The latter is my personal opinion based on versioning doc.

Proposal

I would like to base my proposal on the following assumptions:

  1. Rakudo is possibly not the only one implementation.
  2. Other implementations might not catch up with new revision releases
  3. Information about what revision a test belongs to must be a part of roast because it defines the language.

What I propose:

  1. spectest.data* files are moving from Rakudo to roast.

  2. spectest.data define all revision-independent tests. I.e. those which must pass any revision implementation.

  3. spectest.data.6.<rev> must contain only tests which won't pass on lower revisions. I.e. spectest.data.6.d tests won't pass with use v6.c;.

  4. Revision-specific tests are cumulative. For example, 6.e must also run tests from 6.d and 6.c. To ensure correct testing each revision-specific test must have the version pragma.

    This statement is based on assumption that old revisions are never dropped and any future v6.t release will handle use v6.c; pragma as expected.

  5. In an exceptional case a test from earlier release must never be run on a compiler implementing a newer release. To allow such exception the test could be marked as invalid in a revision-specific spectest.data.

  6. What tests are to be ran is up to particular compiler build subsystem. Rakudo would determine the set of tests based on tools/templates/PERL6_SPECS file where supported revisions are listed.

With the proposed scheme VERSION file from roast becomes obsolete. I consider it redundant.

@vrurg
Copy link
Contributor Author

vrurg commented May 27, 2019

Another consideration I would like to throw in. Roast is currently full of fudging rules tightly bound to rakudo implementation. This is not correct from the perspective of other potential implementations. I think these rules are to be taken out of roast. A replacement for this can be fudge.data files (fudge.data.js for example) containing rules of the following form:

todo:
S02-literals/numeric.t 23,24 "Unsure of what's acceptable for val()"

skip:
S02-literals/radix.t 140 "RT #123862 - negative radix"

Numbers define lines in the test file to be marked as todo/skip. Though this kind of format could be somewhat harder to maintain but it would keep roast free of implementation-specific garbage.

@ugexe
Copy link
Contributor

ugexe commented May 28, 2019

If lines are defined in a separate file then anytime anyone edits any roast file they would have to check and possibly fix the lines file (even for unrelated changes).

@vrurg
Copy link
Contributor Author

vrurg commented May 28, 2019

@ugexe I was looking into it last couple of hours and agree that it was a premature thought.

A better solution would be to have all tests uniquely identified within each file. For example, required a unique sequence to be included into reasons:

ok $succeed, "cover [GH#4242.1]";
is $out, 42, "cover [GH#4242.2]";

A then treat each tests result based on wether the sequence matches to a pattern.

But I'm probably over-complicating now...

@jnthn
Copy link
Contributor

jnthn commented May 29, 2019

Lots to address in this thread; will try and pick off various of the points. One easier one first.

Another consideration I would like to throw in. Roast is currently full of fudging rules tightly bound to rakudo implementation. This is not correct from the perspective of other potential implementations. I think these rules are to be taken out of roast.

Other in-progress implementations are free to add their own fudge rules as needed (and that's happened in the past when they were other actively maintained implementations). The spectests are a shared resource between implementations.

A replacement for this can be fudge.data files (fudge.data.js for example) containing rules of the following form

I think it'll be annoying to maintain those separately. Today if somebody adds a new test, they can put it wherever in the file it feels most appropriate. In doing so, they would change the test numbers, and break the fudge file content.

Overall, I think this suggestion tackles a problem I've not seen us having, by making it more annoying to do the things we already do.

@jnthn
Copy link
Contributor

jnthn commented May 29, 2019

it looks like currently each language revision needs its own individual spectest.data.6. which would largely duplicate the default spectest.data file

Well, copying isn't per-se a problem in that everything of the form spectest.data.6.<rev> would be immutable: released language versions only change so far as errata goes, and those changes are generally small and within a file, not adding/removing entire files. But...

Information about what revision a test belongs to must be a part of roast because it defines the language.

Language releases are handled with tags/branches, so if one wants to test against, say, 6.c, then it would be against the errata branch of the 6.c language version. I don't really see why branches for a release should carry files that are not part of the spec at all, though if we feel deleting all tests not part of that spec release isn't the way to go, then indeed I'd agree that the information about which test files count indeed belongs in roast.

I suspect that there's an amount of stuff in roast that is speculative, and will likely never be implemented; it might well be worth a pass over the files that aren't in spectest.data to see if we want to drop them. A far larger task would be reviewing fudges for the same.

@jnthn
Copy link
Contributor

jnthn commented May 29, 2019

Revision-specific tests are cumulative. For example, 6.e must also run tests from 6.d and 6.c.

I think the most natural way to do this is to go through each of the release errata branches and and run the tests there. That's probably a bit burdensome for everyone to be doing locally, but it could easily surely done as part of CI.

@vrurg
Copy link
Contributor Author

vrurg commented May 29, 2019

@jnthn Thanks!

As to fudging – yes, I rejected the idea already.

Branching could be a little bit of a problem when it comes to bug fixing. Say, a ticket gets closed with a couple of new spectests. Shall we push them back to all revision branches?

A single branch with versionized spectests.data wouldn't have this problem. If a new test fixes a bug in 6.d implementation – this is the only place where something gets changed. Most of the time the only thing would happen is a test file edited. Branching would require cherry-picking into 1,2,... other branches.

From the organizational point of view there is uncertainty as to how to take care of currently active release and an upcoming PREVIEW? Their branched would have to be maintained in parallel and this adds to the complexity. Whereas with versioned files it's easier to maintain control. For example:

  • spectest.c.data, spectest.d.data – these change only upon bugfixes if a new test file is introduced; else not changed at all.

  • spectest.e.PREVIEW.data – experimental features. The file changes actively during development cycle.

  • spectest.e.data – entries are moving here from spectest.e.PREVIEW.data as soon as corresponding developments are finalized and features are accepted as part of the next release.

    Note that .PREVIEW.data/.data approach also allows for good control over the release progress.

Otherwise branching would simplify implementation a lot. For example, to prepare upcoming 6.e support all I'd need to do is change a line in Makefile-common-rules.in and add a macro for that. This would clone the right roast's revision branch.

@vrurg
Copy link
Contributor Author

vrurg commented May 29, 2019

BTW, in either case it looks like spectest.data needs to be moved from rakudo to roast.

@jnthn
Copy link
Contributor

jnthn commented May 29, 2019

Branching could be a little bit of a problem when it comes to bug fixing. Say, a ticket gets closed with a couple of new spectests. Shall we push them back to all revision branches?

No, that was never the intention. Language releases would ideally be tags - that is, immutable - that that's the end of it. In reality, occasionally we might decide it's simply not realistic/worth our while to provide back-compat when we make a breaking fix, and the errata branches let us track those places we've knowingly broken our "promise" of what 6.c meant. I imagine with successive releases, the spectest suite will be gradually in better shape, and we'll have a wider deployed base and less room to simply say "eh, it's not worth it".

Generally, though, the aim is that a language release is immutable, and that - in the presence of a declaration to use that version - we behave in the way that version was specified.

A single branch with versionized spectests.data wouldn't have this problem

But I think this doesn't handle the case where, say, 6.e makes a breaking change, thus invalidating a test that should pass in the presence of a use v6.d. Part of the point of this is that we can make breaking changes, at language boundaries, provided we - in the presence of a use v6.d - provide the previous behavior.

From the organizational point of view there is uncertainty as to how to take care of currently active release and an upcoming PREVIEW?

Well, the intended approach is that if it's a release, it's tagged (maybe with an errata branch), and master represents the speculative progress towards the next language release.

Their branched would have to be maintained in parallel and this adds to the complexity.

In a "the spectests are the set of behaviors you can depend on working" view of the world, if we fix a bug and there wasn't a test covering it in 6.d, then doing that thing correctly is simply not required for 6.d compliance.

@vrurg
Copy link
Contributor Author

vrurg commented May 29, 2019

Ok, I've seen the word, which gives me the ultimate answer. And the word is immutable.

Then, if there're no objections, I'm gonna move spectest.data into roast anyway as it clearly belongs these as a snapshot of release test set. There is spectest.js.data which is implementation dependent and could stay in rakudo though I'd prefer it to be implemented with fudge.

Otherwise I think this thread could be closed.

Great thanks @jnthn!

@jnthn
Copy link
Contributor

jnthn commented May 29, 2019

Ok, I've seen the word, which gives me the ultimate answer. And the word is immutable.

Glad I managed to find a way to convey what was intended.

Then, if there're no objections, I'm gonna move spectest.data into roast anyway as it clearly belongs these as a snapshot of release test set. There is spectest.js.data which is implementation dependent and could stay in rakudo though I'd prefer it to be implemented with fudge.

I think I'm good with the spectest.data moving, and the implementation-dependent override (like spectest.js.data) living on in the Rakudo repo. As someone who as implemented two new Rakudo backends in the past, I can say that a whitelist of tests I wanted to run felt very useful, and I don't think a blacklist of those I didn't would have been as practical - at least, not until up to a point where things were quite close to working.

@vrurg vrurg closed this as completed May 29, 2019
@AlexDaniel
Copy link
Member

Should the result of this discussion be documented in some way?

@vrurg
Copy link
Contributor Author

vrurg commented May 30, 2019

Let me finalize rakudo/rakudo#2852 and I'll add a section into roast/release-guide.md. Just kick me if I forget to... Perhaps it worth re-opening the issue for this purpose.

@vrurg vrurg reopened this May 30, 2019
@vrurg
Copy link
Contributor Author

vrurg commented May 30, 2019

I would suggest adding a label documentation needed to problem solving – similar to tests needed.

@AlexDaniel
Copy link
Member

I would suggest adding a label documentation needed to problem solving

We'll see later if that is something we'll need often. For now I don't feel like adding any secondary labels.

@vrurg
Copy link
Contributor Author

vrurg commented Jun 27, 2019

This problem can be considered solved, relevant changes were merged into the roast and rakudo.

@vrurg vrurg closed this as completed Jun 27, 2019
@AlexDaniel
Copy link
Member

@vrurg that's very nice. Is there enough documentation? For example, if I want to add a 6.e-specific test, which document should I read before doing so?

@vrurg
Copy link
Contributor Author

vrurg commented Jun 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language Changes to the Raku Programming Language
Projects
None yet
Development

No branches or pull requests

4 participants