Skip to content

t/run/todo.t Add list of tickets we don't need tests for #23518

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

Merged
merged 2 commits into from
Aug 6, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 77 additions & 15 deletions t/run/todo.t
Original file line number Diff line number Diff line change
@@ -1,4 +1,81 @@
#!./perl

# This file is a place for tests that are failing at the time they are added
# into this file. It exists so that anyone can contribute a test without
# having to know about Perl's testing internal structures.
#
# These introductory comments include hints that may be revised from time to
# time as we gain experience with what sorts of things people find confusing.
# Therefore it is a good idea to check what's changed since the last time you
# looked.
#
# To add a test, create a new
# TODO: {
# local $::TODO = "GH #####";
# ...
# }
#
# block, like the ones already here. We want to keep the blocks sorted by
# GitHub issue number so that it is easier to check if there already is a test
# for the one you are intending to add.
#
# This file uses the test functionality from t/test.pl. For the most part,
# these look like the ones that Test::More offers, 'is', 'like', and so forth,
# along with a few extras to handle the case where the failure crashes the
# perl interpreter. The ones whose names start with 'fresh' require a
# significant amount of sophistication to use correctly. It's best to start
# out, if possible, by avoiding issues that crash the interpreter and need
# these.

# Some domains have infrastructure which may make it easier to add a test
# there, than to have to set up things here. These include:
#
# Domain Test File
# deparsing lib/B/Deparse.t
# regex matching t/re/re_tests
#
# Before you add a test here, check that the ticket isn't one of these,
# because we already have todo tests for them (in some other file).
#
# Git Hub issue numbers
Comment on lines +37 to +40

Choose a reason for hiding this comment

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

I'm not sure if this really is the best approach..

My expectation is that the list of issue numbers will become out-of-sync and won't be updated when new todo tests are added in other files..

A possible alternative: update the TODO-tests that uses RT/perl numbers and replace (or add) the GitHub numbers and change this to include something like:

Before adding a test check that a TODO test does not yet exist by running: git grep "GH ..." t

(A starting point for the old RT numbers: git grep -E 'TODO.*(perl|RT)[[:space:]]*#' t/ dist/ ext/)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still think it is the best approach at this time. I will be watching for this getting outdated and take steps to make sure it doesn't. And that won't require very much work as it is quite rare for a TODO test to get added that isn't quickly removed. One set was added by @iabyn a year ago as a result of his reworking the XS parsing. The previous instance was a year before that via 981e5d6

I'm now reluctant to use GH as a prefix to ticket numbers, as it seems to break their automatic closing by the system when a p.r. gets merged that refers to a ticket with that prefix.

The list was created by me by going through the code, looking for every extant use of todo. It should be complete as of now, and no one is going to add anything new for things that use non-GH numbering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And I'm trying to make this as simple as possible so people thinking of contributing have as little extra steps to take as possible

# 2207
# 2208
# 2286
# 2931
# 4125
# 4261
# 4370
# 5959
# 8267
# 8945
# 8952
# 9010
# 9406
# 10750
# 14052
# 14630
# 19370
# 19661
# 22547
#
# We keep a list of all the people who have contributed to the Perl 5 project.
# If this is your first time contributing, you will need to add your name to
# this list. After you have changed this file with your new test and
# committed the result, run
#
# perl Porting/updateAUTHORS.pl
#
# This will automatically add you (if you weren't there already) to our list
# of contributors. If so, you will need to commit this change by doing
# something like:
#
# commit -a -m'[your name here] is now a Perl 5 author'
#
# Adding tests here helps in two ways. It might show that the bug has already
# been fixed and we just don't know it; or skimming the existing tests here
# might show that there is an existing ticket already open, and the new ticket
# can be marked as duplicate.

BEGIN {
chdir 't' if -d 't';
require './test.pl'; # for fresh_perl_is() etc
Expand All @@ -8,24 +85,9 @@ BEGIN {
}

use Config;

use strict;
use warnings;

# This file is a place for tests that fail at the time they are added here.
#
# When a ticket is filed, just follow the paradigm(s) in this file to add a
# test that shows the failure.
#
# It is expected that when new tickets are opened, some will actually be
# duplicates of existing known bad behavior. And since there are so many open
# tickets, we might overlook that. If there is a test here, we would
# automatically discover that a fix for the newer ticket actually fixed an
# earlier one (or ones) as well. Thus the issue can be closed, and the final
# disposition of the test here determined at that time. (For example, perhaps
# it is redundant to the test demonstrating the bug that was intentionally
# fixed, so can be removed altogether.)

my $switches = "";

my $is_debugging_build = $Config{cppflags} =~ /-DDEBUGGING/;
Expand Down
Loading