Remove expected key - Githubissues

iHiD commented 3 years ago

It has become clear that having a separate expected key is causing too much pain for tracks. This PR changes the spec to remove it. I think this:

"negatively" affects @neenjaw @thelostlambda @ErikSchierboom and me in terms of invalidating code we've written (sorry 💙)
Makes everyone else's lives easier.
Means that tracks can still have multiple assertions (although I think that there is risk they can still be confusing for students in situations where there are multiple assertions in a test and it's unclear which is failing - so please tread carefully).
Means that we can probably do most of the work using AWK (or similar) rather than AST parsing.

Feedback welcome. Sorry this has been such a ridiculously painful journey.

iHiD commented 3 years ago

Yes. Exactly.

neenjaw commented 3 years ago

Rather than cmd can we call this body or test_body or something since it may contain block of code within the test in addition to the assertion expression?

Also, it would avoid confusion between this and the previous cmd key value, allowing later progressive enhancement of this feature seamlessly

SaschaMann commented 3 years ago

I think expression would fit well

iHiD commented 3 years ago

Yes. Happy to rename.

test_code maybe?

I don't love just body (too generic) or expression (doesn't imply to me that a complex multi-part piece of code would be allowed). I'd be ok with test_body too. Whatever we choose, I think it should be multi-(programming)-lingual. I'm not sure if "body" is that? (I don't think expression is either as I've never used that term in Ruby)

neenjaw commented 3 years ago

test_body would be ok

ErikSchierboom commented 3 years ago

I'm in favor of test_body or even test_code.

edit: I just noticed that this PR actually uses "Test Code" in its UI example, so maybe that is the right suggestion?

coriolinus commented 3 years ago

If we're copying the test code into test_code, then we shouldn't be manually populating that field per-test at all. Instead, we can populate a regular expression or something once per track, with a once-per-exercise optional override, which extracts per-test code.

Anything more manual than that adds maintenance burden, and is begging for things to get out of sync.

ErikSchierboom commented 3 years ago

If we're copying the test code into test_code, then we shouldn't be manually populating that field per-test at all. Instead, we can populate a regular expression or something once per track, with a once-per-exercise optional override, which extracts per-test code.

Anything more manual than that adds maintenance burden, and is begging for things to get out of sync.

We're not expecting tracks to do this manually (which would be a very bad idea indeed), but instead expect the test runners to handle this complete automatically. Which is also what @neenjaw @TheLostLambda and I were already doing in our proof of concepts.

iHiD commented 3 years ago

Anyone hate test_code or can we go with that?

iHiD commented 3 years ago

Going to merge this as it is. Will rename cmd in a follow up shortly to give anyone time to object :)

exercism / automated-tests

Remove expected key #58