Przeglądaj źródła

Ensure we skip_all when replaying with a missing mock_file

When refactoring the test harness setup into a separate lib, I made a
bug where OS types that we weren't prepared for would fail the tests
instead of skipping everything. CPAN testers found this by running a
test on cygwin windows.

cpan tester report on cygwin:
http://www.cpantesters.org/cpan/report/a87063d1-6e72-1014-bdc8-69e85cf4dae8

Because of the way the `t/lib/TestHarness.pm` was set up, we were never
getting to the `-e $mock_file` check that would use the `plan skip_all
=> ...` to allow the tests to pass when we don't have a mock
prepared. Instead, we were going straight into Mock::RemoteConnection
where it would rightly croak due to having non-existent file.

On the upswing, I found that we _can_ indeed do `plan skip_all => ...`
from TestHarness.pm instead of the `.t` file itself, so that's
nice. We're able to clean up the set up for each test down to one
command:

```perl
my %selenium_args = %{ TestHarness->new(
    this_file => $FindBin::Script
)->base_caps };
```

While it is pleasingly DRY, unfortunately, it's not very obvious what's
going on in there. That's not good. It's much more dry than the old
method, but none of that code indicates that we're deciding whether or
not to record, finding the mock files, etc, etc.
Daniel Gempesaw 11 lat temu
rodzic
commit
35c4589d00
1 zmienionych plików z 9 dodań i 19 usunięć
  1. 9 19
      t/lib/TestHarness.pm

+ 9 - 19
t/lib/TestHarness.pm

@@ -103,28 +103,18 @@ has mock_file => (
         my $test_name = lc($self->calling_file);
         $test_name =~ s/\.t$//;
 
-        return $mock_folder . $test_name . '-mock-' . $self->os . '.json';
-    }
-);
+        my $mock_file = $mock_folder . $test_name . '-mock-' . $self->os . '.json';
 
-sub skip_all_unless_mocks_exist {
-    my ($self) = @_;
-    unless ($self->mocks_exist_for_platform) {
-        plan skip_all => "Mocking of tests is not been enabled for this platform";
-    }
-}
+        # If we're replaying, we need a mock to read from. Otherwise,
+        # we can't do anything
+        if (not $self->record) {
+            plan skip_all => "Mocking of tests is not been enabled for this platform"
+              unless -e $mock_file;
+        }
 
-sub mocks_exist_for_platform {
-    my ($self) = @_;
-    if ($self->record) {
-        return 1;
-    }
-    else {
-        # When we're replaying a test, we need recordings to be able
-        # to do anything
-        return -e $self->mock_file;
+        return $mock_file;
     }
-}
+);
 
 sub DEMOLISH {
     my ($self) = @_;