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

New test format #766

Merged
merged 1 commit into from
Apr 12, 2018
Merged

New test format #766

merged 1 commit into from
Apr 12, 2018

Conversation

codebybrett
Copy link

This commit changes encoding of individual test vectors to GROUP! and repurposes BLOCK! to be a containing test group.
Bug references previously encoded in comments and in ISSUE! are now consistently encoded as an ISSUE! that begins a test group.
Flags are now encoded as TAG!

The conversion code is temporarily left built-in to the test scripts so that developers can upgrade their own code bases.
The test parser has been upgraded to emit token types for precision, documentation and in case further work is to be done on it.

This commit changes encoding of individual test vectors to GROUP! and repurposes BLOCK! to be a containing test group.
Bug references previously encoded in comments and in ISSUE! are now consistently encoded as an ISSUE! that begins a test group.
Flags are now encoded as TAG!

The conversion code is temporarily left built-in to the test scripts so that developers can upgrade their own code bases.
The test parser has been upgraded to emit token types for precision, documentation and in case further work is to be done on it.
@hostilefork
Copy link
Member

hostilefork commented Apr 9, 2018

I merged it and all tests passing, looks good!

Since you've got the conversion all running, I guess right now would be the best time for us to think about any potential tweaks. Your tests here were the motivator, so I'd like to get the ones that work in and raise the others as issues:

https://github.com/codebybrett/reb/blob/master/test/dates.test.reb

For your "setup" code you just use an ordinary BLOCK! (which would be a GROUP! now) and put "true" at the end of it. Which works, but it feels like there should be another way of saying that.

I pointed out in chat that there is this the old, common pattern of doing some setup and then testing a number of conditions:

[ ;-- #whatever, NOTES
    foo: func [...] [...]
    more-setup-code
    did all [
        [...] = foo ...
        [...] = foo ...
    ]
]

If one of those conditions returns false, you have to manually go in and see which one. So I thought it better to divide this where it could indicate which failed. Then I suggested ELIDE instead of the true at the end of "setup code".

[
    #whatever {Notes}
    elide (
        foo: func [...] [...]
        more-setup-code
    )
    ([...] = foo ...)
    ([...] = foo ...)
]

It makes me wonder if we should use some other kind of divider as a signal, then just not use those dividers in the tests? :-/

[
    #whatever {Notes}
    ---
    foo: func [...] [...]
    more-setup-code
    ---
    ([...] = foo ...)
    ([...] = foo ...)
]

Of course it could be --> and <-- or some other choice, or === and ---.

This kind of thing would go a little more in the direction of Red's tests without going quite all the way there. I do like the BLOCK! and the GROUP!s, and the ability to put GROUP!s on their own outside blocks. Red's tests are rather noisy to look at, but by not using BLOCK! and GROUP! they are visually harder to "mix up with Rebol code" by being so meta.

Though true isn't bad, necessarily. It keeps things in the spirit of simplicity, I guess.

[
    #whatever {Notes}
    (
        foo: func [...] [...]
        more-setup-code
        true
    )
    ([...] = foo ...)
    ([...] = foo ...)
]

Anyway, just a good moment to ask what the best answer is....if we have one. If you don't think it affects the conversion either way, we can just merge things in as they are.

One thing to weigh in is that test frameworks that get more and more meta are able to give better reports. If you say [#242 equal 10 (1 + 3 + 5)] it can come back and say "expected 10, got 9" in the log and save you some time. Not sure how much of that is in Red's tests, might be worth a look...

@codebybrett
Copy link
Author

If no-one is objecting to the change my preference is to not defer this commit for a few reasons. Since any proposed setup code inside the BLOCK! is optional, catering for setup shouldn't change the existing conversion. I don't think the conversion can do much more and delaying may cause re-work. Time-wise, I spent quite a while to dot the i's and cross the t's on this conversion so I'm planning on putting this aside right now to work on stuff I've neglected recently.

I realise the point of this excercise is to get added value from grouped tests, but I think this should be a separate phase and perhaps it might be good to let these changes to settle in a bit, some time with them may make things clearer about what would be nice.

I'm unsure what would constitute a decent setup format. Maybe just a BLOCK! before the first GROUP!, or put a keyword "context" before it - "here is the context for these tests".

[
    #whatever {Notes}
    context [
        foo: func [...] [...]
        more-setup-code
    ]
    ([...] = foo ...)
    ([...] = foo ...)
]

Presumably any setup will be shared/mutable between the tests, rather than re-setup fresh for each test. Or perhaps that's a different keyword.

@hostilefork hostilefork merged commit 748ab9c into metaeducation:master Apr 12, 2018
@hostilefork
Copy link
Member

Your preference is my preference. :-) Okay, it's done...thanks, on we go!

@codebybrett codebybrett deleted the grouped-tests branch April 12, 2018 07:25
@hostilefork
Copy link
Member

hostilefork commented Apr 20, 2018

I know you've got other things to do, but... I'm sure you don't want to miss out on trying CONCOCT!

>> concoct (()) [(1 + 2) ((1 + 2))]
== [(1 + 2) 3]

>> concoct [] quote ((1 + 2) [3 + 4] (5 + 6))
== ((1 + 2) 7 (5 + 6))

>> concoct [[]] 'a/(b)/[[1 + 2]]/[c]/d
== a/(b)/3/[c]/d

As the Trello card points out, it can be useful even if it's not strictly necessary, in non-/DEEP situations...just for clarity. Might need a better name.

(Also it would really help if people could get in some basic tests now that Latin-1 Nowhere is committed...)

Tests going well so far... just got to figure out how to run them in "isolated environments" :-/

@codebybrett
Copy link
Author

Concoct [looks like you dug deep into the names barrel for that one ;-) ] looks good. Composing with literal groups has always been painful. The structure based replacement looks very cool.

Concoct reminds me that search and rewrite or similar should probably be part of the base toolkit too.

And I'm also reminded of the brief discussion of prolific binding. Many blocks have bindings quite irrelevent to the intent of the block, to the extent that being able to flag "leave unbound except for x.." would be nice in preventing unexpected errors due to binding. I wondered whether if compose was such a flag and whether that would be useful. I know that's a can of worms and probably on your considerations list anyway.

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

Successfully merging this pull request may close these issues.

2 participants