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

test: Rename TEST_OUTPUT to COMPILE_OUTPUT #1579

Conversation

leandro-lucarella-sociomantic
Copy link
Contributor

The real meaning for TEST_OUTPUT is to provide the correct output from the
compiler, not the whole test. This is specially confusing for runnable
tests where someone could think this test parameter is used to check
against the program output. For this reason the test parameter is renamed
to COMPILE_OUTPUT (which is the same internal name d_do_test.d use).

Also the documentation in test/Makefile is updated to describe the
previously missing test parameter.

The real meaning for TEST_OUTPUT is to provide the correct output from the
compiler, not the whole test. This is specially confusing for runnable
tests where someone could think this test parameter is used to check
against the program output. For this reason the test parameter is renamed
to COMPILE_OUTPUT (which is the same internal name d_do_test.d use).

Also the documentation in test/Makefile is updated to describe the
previously missing test parameter.
@ghost
Copy link

ghost commented Feb 2, 2013

A whole lot of changes for little benefit. Also notice there's over 100 pull requests opened - all the test-cases which depend on TEST_OUTPUT will have to be fixed.

@9rnsr
Copy link
Contributor

9rnsr commented Feb 2, 2013

@leandro-lucarella-sociomantic , As @AndrejMitrovic already says, changing the poor name TEST_OUTPUT is little benefit.
But, I think adding documentation in test/Makefile is still valuable.

Could you stop just only the renaming?

@9rnsr
Copy link
Contributor

9rnsr commented Feb 2, 2013

Or, support "TEST_OUTPUT" and "COMPILE_OUTPUT" at the same time. If it is possible, we will not break opened pull requests.

@leandro-lucarella-sociomantic
Copy link
Contributor Author

On Fri, Feb 01, 2013 at 11:01:41PM -0800, Hara Kenji wrote:

@leandro-lucarella-sociomantic , As @AndrejMitrovic already says, changing the poor name TEST_OUTPUT is little benefit.
But, I think adding documentation in test/Makefile is still valuable.

I understand, but this triggers another problem I see with the current
auto-tester and this metadata attached to the tests: if you have a typo,
you'll never find out.

It would be nice to have at least a warning to be printed (although I
think a plain error should be even better) if one of these "keywords"
are found in a test but is unknown.

If we did this, any outdated test will just fail if they have the old
keyword.

I'll take a look to see how doable is this...

Could you stop just only the renaming?

Yeah, sure, meanwhile I will just post a pull request with the
documentation, I can post the other stuff as a separate pull request if
I get it right.

@leandro-lucarella-sociomantic
Copy link
Contributor Author

On Fri, Feb 01, 2013 at 11:15:19PM -0800, Hara Kenji wrote:

Or, support "TEST_OUTPUT" and "COMPILE_OUTPUT" at the same time. If it
is possible, we will not break opened pull requests.

What do you think about rejecting COMPILE_OUTPUT (or any other unknown
keyword)?

@leandro-lucarella-sociomantic
Copy link
Contributor Author

See #1613

@leandro-lucarella-sociomantic
Copy link
Contributor Author

On Mon, Feb 04, 2013 at 05:26:07PM +0100, Leandro Lucarella wrote:

On Fri, Feb 01, 2013 at 11:01:41PM -0800, Hara Kenji wrote:

@leandro-lucarella-sociomantic , As @AndrejMitrovic already says, changing the poor name TEST_OUTPUT is little benefit.
But, I think adding documentation in test/Makefile is still valuable.

I understand, but this triggers another problem I see with the current
auto-tester and this metadata attached to the tests: if you have a typo,
you'll never find out.

It would be nice to have at least a warning to be printed (although I
think a plain error should be even better) if one of these "keywords"
are found in a test but is unknown.

This seems to be quite impossible with the current implmementation of
d_do_test. Keywords are just scanned through the whole file without
any parsing at all, so just trying to find things that look like
keywords seems too weak, unless at least we add some special token to
identify keywords, something like @COMPILE_OUTPUT@: instead of just a
plan COMPILE_OUTPUT:.

Here is a pull request (#1615) as a proof of concecpt, if you think this
is acceptable, I can update this one to use the new syntax which makes
tests with unrecognized keywords just fail.

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