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

comp/parser.t count two tests #18308

Closed
wants to merge 3 commits into from
Closed

comp/parser.t count two tests #18308

wants to merge 3 commits into from

Conversation

brainbuz
Copy link
Contributor

Two lines in comp/parser.t were present to make sure they didn't crash the parser.
I enclosed the relevant section in a block {.
I added Pass tests immediately after each of the two lines.
Motiviation: I was working on Perl 7 Alpha tests, this section of the code generates warnings that need silencing and it is easier to see all the related code and comment when the section is grouped together in a block. Since each of the two lines is testing something it should be counted as a test.

…rashed

parser as tests (PASS if the test file is still running after the lines).
@Grinnz
Copy link
Contributor

Grinnz commented Nov 10, 2020

Not sure I see the point of the block, Nothing within it is lexically scoped.

3 similar tests eval a sub with a list of variables, $r is repeated at the
end of the list, but the errors that are being checked have nothing to do with
the repeated variable. This causes a warning enabled.
@brainbuz
Copy link
Contributor Author

brainbuz commented Nov 12, 2020

Found what I think is a typo in parser.t adding to pull request. The typo causes 'redefined' warnings when warnings are enabled.

The point of the block is that for Perl 7 some warnings need to be disabled, for Perl 5 the only purpose is it groups the code making it more readable. The lines that I added tests for should be counted as tests.

@Grinnz
Copy link
Contributor

Grinnz commented Nov 12, 2020

I don't know what that means. There are no warnings being disabled here. Also, it is unknown what Perl 7 will entail.

@brainbuz
Copy link
Contributor Author

I've removed the braces.

@tonycoz
Copy link
Contributor

tonycoz commented Nov 23, 2020

Rather than is(1, 1, ...) wouldn't they be better as pass(...)?

@brainbuz
Copy link
Contributor Author

The test doesn't use test::more or test.pl and doesn't have a pass routine defined.
If it is better form to add a pass subroutine I can do that.
something like sub pass { is( 1,1, @_ ) } should work

@tonycoz
Copy link
Contributor

tonycoz commented Nov 23, 2020

The test doesn't use test::more or test.pl and doesn't have a pass routine defined.
If it is better form to add a pass subroutine I can do that.
something like sub pass { is( 1,1, @_ ) } should work

Ahh, you're right, sorry for the noise.

I've squashed the braces change into the base commit, rebase and applied your changes to blead as 4281700 and a5fba4f.

@tonycoz tonycoz closed this Nov 23, 2020
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.

3 participants