SPT: Cannot handle top-level tuple in run strategy expectation.
In https://code.studioinfinity.org/glen/spoofax_prop, I have a working concrete syntax for the propositional language in the Stratego Tutorial/Reference Manual, and I have set up Stratego files
prop-dnf3.str
andprop-cnf3.str
for the functionalized disjunctive and conjunctive normal form examples of Section 5.1.2 of the manual, including the example that computes both simultaneously by reducing the pair (Dnf(x), Cnf(x)). They work fine via an ESV menu item, but the SPT test (quoted below) that I have created for the “both at once” example fails. It complains of a syntax error, but runs anyway, and then fails because it is confused about the AST it should expect – it appears to have interpreted the “to” keyword as a binary constructor, part of the expected AST, rather than as an SPT keyword with a tuple for the expected AST. This appears to be a “bug” in the SPT language definition.test sec5_1_2_test1_both_ex [[ (r -> p & q) & p ]] run dcnf to (Or(And(Not(Atom("r")),Atom("p")),And(And(Atom("p"),Atom("q")),Atom("p"))), And(And(Or(Not(Atom("r")), Atom("p")), Or(Not(Atom("r")), Atom("q"))), Atom("p")))
The errors reported are:
Syntax error, expected: ‘to’ [on the third line of this example]
The result of running dcnf did not match the expected result.
Expected: to(Or([…]), And([…]))
Got: (Or([…]),And([…]))[where all of the elided […] match exactly what’s shown in the corresponding spots in the test case.] In the repository, this test is in the file
Submitted by Glen Whitney on 17 January 2021 at 17:58test/manual-suite.spt
.
Issue Log
This seemed like an oversight in SPT that would be relatively easy to fix, so I gave it a shot and looks like I have a basic fix ready (https://github.com/metaborg/spt/pull/36). It will need review and testing, which takes more time that I don’t have right now, but at least it’s work in progress.
As a workaround for now you could change the output of dcnf to give you a constructor called
Pair
or something like that and match against that in the SPT test…
That’s great! My goal in spoofax_prop is to maintain the greatest fidelity to the Spoofax Tutorial/Reference as possible, so for now I’ve just added to my commentary that there is a fix for this in progress. When it eventually lands, I will remove the note about the test failing from spoofax_prop.
I just pushed the changes for support of tuples in SPT, I believe the buildfarm will pick up the change in build 549, so once that build is finished you can try it out in the development release of Spoofax.
Yup, works for me now. I would close this issue, but I don’t see an option for that. Thanks!Actually, when I select my “manual-suite.spt” and choose the menu option “Spoofax (meta) > Run all selected tests” I get an alert
'RunTestsJob' has encountered a problem. An internal error occurred during: "RunTestsJob". Details: An internal error occurred during: "RunTestsJob". Index out of bounds: 1
And also all of the tests prior to the one with tuples are reported as having run, and it and all of the ones after it are reported as having not yet run. So it seems as though actually this is not quite out of the woods yet…
And if I comment out the test with tuples, all of the tests run. So the test with tuples definitely still seems to be the culprit. (But it no longer generates a parse error, and the “to” keyword is properly highlighted.)
Hmm, yes, based on that I was quickly able to find the problem. I pushed a hotfix, so let’s see how the next build does…
But this also means the tests I wrote didn’t actually get executed during the build :( I’ll open another issue to track that.
All seems well on this with the Jan 21 build. So I reinstate my comment that this issue could be closed.
Thanks! Glen
Good to hear
Log in to post comments