Skip to content

Provide explicit undef to indicate TODO condition is not met#30

Open
jkeenan wants to merge 1 commit into
masterfrom
use-undef-in-local-todo
Open

Provide explicit undef to indicate TODO condition is not met#30
jkeenan wants to merge 1 commit into
masterfrom
use-undef-in-local-todo

Conversation

@jkeenan

@jkeenan jkeenan commented Jan 20, 2019

Copy link
Copy Markdown

... rather than implicit empty string (false but defined).

Reflects change in Test-Simple distribution 1.302160, which is now
incorporated into perl-5.29.7.

See: Test-More/test-more@9c269ff

... rather than implicit empty string (false but defined).

Reflects change in Test-Simple distribution 1.302160, which is now
incorporated into perl-5.29.7.

See: Test-More/test-more@9c269ff
Comment thread t/extract-version.t

if (ref($expected_version) eq 'CODE') {
local $TODO = $test_case->{TODO_code_sub};
local $TODO = $test_case->{TODO_code_sub} || undef;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line shouldn't need to be changed - $test_case->{TODO_code_sub} is never defined-but-false.

Comment thread t/extract-version.t
}
else {
local $TODO = $test_case->{TODO_scalar};
local $TODO = $test_case->{TODO_scalar} || undef;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as TODO_code_sub.

Comment thread t/extract-version.t

if (exists $test_case->{all_versions}) {
local $TODO = $test_case->{TODO_all_versions};
local $TODO = $test_case->{TODO_all_versions} || undef;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as TODO_code_sub.

Comment thread t/extract-version.t
# explicitly check the stringified form.
{
local $TODO = !defined($got) && ($test_case->{TODO_code_sub} || $test_case->{TODO_scalar});
local $TODO = !defined($got) && ($test_case->{TODO_code_sub} || $test_case->{TODO_scalar}) || undef;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more strict than the original, but if the change is consequential it will result in more tests failing due to a lack of a TODO_* where there should be one, so it should make the problem more apparent. (That is, I think this change is not harmful.)

@karenetheridge karenetheridge left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an entry to Changes, as per the travis failures.

@jkeenan

jkeenan commented Jan 20, 2019 via email

Copy link
Copy Markdown
Author

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