Skip to content

Commit

Permalink
update cookie test to only test against test cookie jar
Browse files Browse the repository at this point in the history
The test cookie jar implementation is incomplete, and the corpus data
matches its behavior, rather than the behavior of a real cookie jar. The
test was previously attempting to optionally test against
HTTP::CookieJar, but due to a bug in the test just tested against the
test cookie jar a second time.

For now, change the test to only test against the test cookie jar. Also
never skip the test cookie jar test (such as for a syntax error). Fix
the test code to work against the specified cookie jar, even though we
are no longer trying to test any others. Even with incorrect data, the
test is still checking HTTP::Tiny's interaction with the cookie jar.

In the future, it would be good to update the test cookie jar to more
closely match the real behavior, fix the corpus data, and re-enable
testing against HTTP::CookieJar.
  • Loading branch information
haarg committed Nov 12, 2024
1 parent 364970f commit 6922800
Showing 1 changed file with 26 additions and 9 deletions.
35 changes: 26 additions & 9 deletions t/160_cookies.t
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,39 @@ use Util qw[tmpfile rewind slurp monkey_patch dir_list parse_case

use HTTP::Tiny;
BEGIN { monkey_patch() }
use SimpleCookieJar;

SKIP: for my $class ( qw/SimpleCookieJar HTTP::CookieJar/ ) {
# XXX: this test is broken. SimpleCookieJar doesn't implement enough to behave
# correctly with the responses in the corpus data. And the requests in the
# corpus match that incorrect implementation, so if used against a real cookie
# jar, it will break. It still tests interactions with the jar object, so it
# is still a valuable test even in its partly broken state.

for my $class (
'SimpleCookieJar',
#'HTTP::CookieJar', # this test doesn't actually work with a real cookie jar
#'HTTP::Cookies', # would be nice to support eventually
) {
(my $module = $class . ".pm") =~ s{::}{/}g;

subtest $class => sub {
eval "require $class; 1"
or plan skip_all => "Needs $class";
eval { require $module; 1 } or do {
die $@
if $ENV{RELEASE_TESTING};
plan skip_all => "Needs $class";
};

for my $file ( dir_list("corpus", qr/^cookies/ ) ) {
my $label = basename($file);
my $data = do { local (@ARGV,$/) = $file; <> };
my @cases = split /--+\n/, $data;

my $jar = SimpleCookieJar->new();
my $jar = $class->new();
my $http = undef;
my $case_n = 0;
while (@cases) {
my ($params, $expect_req, $give_res) = splice( @cases, 0, 3 );
$case_n++;

my $case = parse_case($params);

Expand All @@ -37,13 +54,13 @@ SKIP: for my $class ( qw/SimpleCookieJar HTTP::CookieJar/ ) {
my %new_args = hashify( $case->{new_args} );

if( exists $headers{Cookie} ) {
my $cookies = delete $headers{Cookie};
$jar->add( $url, $cookies );
my $cookies = delete $headers{Cookie};
$jar->add( $url, $cookies );
}

if( exists $headers{'No-Cookie-Jar'} ) {
delete $headers{'No-Cookie-Jar'};
$jar = undef;
delete $headers{'No-Cookie-Jar'};
$jar = undef;
}

my %options;
Expand All @@ -70,7 +87,7 @@ SKIP: for my $class ( qw/SimpleCookieJar HTTP::CookieJar/ ) {
my $response = $http->get(@call_args);

my $got_req = slurp($req_fh);
is( sort_headers($got_req), sort_headers($expect_req), "$label request data");
is( sort_headers($got_req), sort_headers($expect_req), "$label case $case_n request data");
}
}
};
Expand Down

0 comments on commit 6922800

Please sign in to comment.