Edit and test warnings added in 6554d96

sqitchers · Apr 16, 2023 · afe87ce · afe87ce
1 parent 1bc3102
commit afe87ce
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 14 deletions.
diff --git a/lib/App/Sqitch/Engine.pm b/lib/App/Sqitch/Engine.pm
@@ -72,9 +72,11 @@ has no_prompt => (
     isa     => Bool,
     trigger => sub {
         # Deprecation notice added Feb 2023
-        warnings::warnif("deprecated",
-                         __("Engine::no_prompt is deprecated and will be removed in a future release\n".
-                            "Use direct arguments to revert() instead"));
+        warnings::warnif(
+            "deprecated",
+            "Engine::no_prompt is deprecated and will be removed in a future release\n"
+            . "Use direct arguments to revert() instead",
+        );
     }
 );
 
@@ -83,9 +85,11 @@ has prompt_accept => (
     isa     => Bool,
     trigger => sub {
         # Deprecation notice added Feb 2023
-        warnings::warnif("deprecated",
-                         __("Engine::prompt_accept is deprecated and will be removed in a future release\n".
-                            "Use direct arguments to revert() instead"));
+        warnings::warnif(
+            "deprecated",
+            "Engine::prompt_accept is deprecated and will be removed in a future release\n"
+            . "Use direct arguments to revert() instead",
+        );
     }
 );
 
@@ -300,15 +304,16 @@ sub revert {
     my ( $self, $to, $prompt, $prompt_default ) = @_;
 
     if (defined $prompt) {
-        hurl revert => __('Missing mandatory parameter $prompt_default') unless defined $prompt_default;
+        hurl revert => __('Missing required parameter $prompt_default')
+            unless defined $prompt_default;
     } else {
-        warnings::warnif("deprecated",
-                         __("This calling style of Engine::revert is deprecated.\n".
-                            "In the future `prompt` and `prompt_accept` parameters will be mandatory"));
-
-        my $no_prompt = $self->no_prompt // 0;
-        $prompt = ! $no_prompt;
+        warnings::warnif(
+            "deprecated",
+            "Engine::revert() requires the `prompt` and `prompt_default` arguments.\n"
+            . 'Omitting them will become fatal in a future release.',
+        );
 
+        $prompt = !($self->no_prompt // 0);
         $prompt_default = $self->prompt_accept // 1;
     }
 

diff --git a/t/engine.t b/t/engine.t
@@ -4,7 +4,7 @@ use strict;
 use warnings;
 use 5.010;
 use utf8;
-use Test::More tests => 769;
+use Test::More tests => 773;
 # use Test::More 'no_plan';
 use App::Sqitch;
 use App::Sqitch::Plan;
@@ -14,6 +14,7 @@ use Test::Exception;
 use Test::NoWarnings;
 use Test::MockModule;
 use Test::MockObject::Extends;
+use Test::Warn qw(warning_is);
 use Time::HiRes qw(sleep);
 use Locale::TextDomain qw(App-Sqitch);
 use App::Sqitch::X qw(hurl);
@@ -1920,6 +1921,19 @@ is_deeply $engine->seen, [
 ], 'It should only have called deployed_changes()';
 is_deeply +MockOutput->get_info, [], 'Nothing should have been output';
 
+# Make sure deprecation warning happens.
+warning_is { $engine->revert }
+    "Engine::revert() requires the `prompt` and `prompt_default` arguments.\n",
+    'Should get warning omitting required arguments';
+is_deeply +MockOutput->get_info, [
+    [__ 'Nothing to revert (nothing deployed)']
+], 'Should have notified that there is nothing to revert';
+is_deeply $engine->seen, [
+    [lock_destination => []],
+    [deployed_changes => undef],
+], 'It should only have called deployed_changes()';
+is_deeply +MockOutput->get_info, [], 'Nothing should have been output';
+
 # Try reverting to an unknown change.
 throws_ok { $engine->revert('nonexistent', 1, 1) } 'App::Sqitch::X',
     'Revert should die on unknown change';