reimplement fish-spec for nicer output (#955)

* replace implementation of fish-spec

provide a new implementation for fish-spec with more verbose and colored
output and more assertion functions

* fix indentation for fish-spec

* properly namespace color echo functions in fish-spec

* use line rewriting for fish-spec for more terse output

* use fish 3.0.0 compatible syntax

* fix assorted assertion bug

* ensure tests actually pass

* fix existing omf spec tests

* simplify assert_in_array and assert_not_in_array

---------

Co-authored-by: Kevin F. Konrad <kevin.konrad@skillbyte.de>

README.md

@@ -203,6 +203,16 @@ It's highly recommended that your custom startup commands go into `init.fish` fi
 
 If you need startup commands to be run *before* Oh My Fish begins loading plugins, place them in `before.init.fish` instead. If you're unsure, it is usually best to put things in `init.fish`.
 
+## Testing
+
+Oh My Fish provides a test framework inspired by RSpec called `fish-spec` via a built-in package. You can use
+`fish-spec` to test your fish package by writing tests in `spec/*_spec.fish` files within your package. To run the tests
+type in `fish-spec`. If your tests are in different files you can also list your test files (or globs of test files)
+like so: `fish-spec tests/test_*.fish other-tests/foo.fish`
+
+For syntax and available assertions see the tests for the fish-spec package in
+[pkg/fish-spec/spec/](https://github.com/oh-my-fish/oh-my-fish/tree/master/pkg/fish-spec/spec).
+
 ## Creating Packages
 
 Oh My Fish uses an advanced and well defined plugin architecture to ease plugin development, including init/uninstall hooks, function and completion autoloading. [See the packages documentation](docs/en-US/Packages.md) for more details.

pkg/fish-spec/basic_formatter.fish

@@ -1,59 +0,0 @@
-function __fish-spec.all_specs_init -e all_specs_init -a spec
-  set -g __fish_spec_start_time (__fish-spec.current_time)
-end
-
-function __fish-spec.all_specs_finished -e all_specs_finished -a spec
-  set -l __fish_spec_end_time (__fish-spec.current_time)
-  set -l diff (math "($__fish_spec_end_time - $__fish_spec_start_time) * 0.001")
-
-  echo -en '\n\nFinished in '
-  printf '%g' $diff
-  echo ' seconds'
-end
-
-function __fish-spec.spec_init -e spec_init -a spec
-  set -g __current_spec_name (echo $spec | sed 's/^[0-9]*_//;s/_/ /g;s/^it/It/')
-  set -e __current_spec_output
-  set -e __current_spec_status
-end
-
-function __fish-spec.spec_finished -e spec_finished -a spec
-  functions -e $spec
-
-  switch "$__current_spec_status"
-    case success
-      emit spec_success
-    case error
-      emit spec_error
-    case '*'
-      emit spec_no_assertions
-  end
-end
-
-function __fish-spec.spec_success -e spec_success
-  echo -n '.'
-end
-
-function __fish-spec.spec_error -e spec_error
-  echo -e "\n\nFailure: $__current_spec_name"
-
-  if not set -q __current_spec_quiet
-    echo (omf::em)  $__current_spec_output(omf::off)
-  end
-
-  set -g __any_spec_failed true
-end
-
-function __fish-spec.spec_no_assertions -e spec_no_assertions
-  echo -n 'N/A'
-end
-
-function __fish-spec_assertion_success -e assertion_success
-  set -q __current_spec_status; or set -g __current_spec_status success
-end
-
-function __fish-spec_assertion_error -e assertion_error -a error_message
-  # Mimics output redirect inside an event handler
-  set -g __current_spec_output $error_message
-  set -g __current_spec_status error
-end

pkg/fish-spec/framework/assertions.fish

@@ -0,0 +1,180 @@
+function __fish_spec_assert_generic -a first second success_template failure_template test_command
+  set __fish_spec_total_assertions_in_file (math $__fish_spec_total_assertions_in_file + 1)
+
+  if eval not "$test_command"
+    set __fish_spec_failed_assertions_in_file (math $__fish_spec_failed_assertions_in_file + 1)
+    set __fish_spec_last_assertion_failed yes
+    eval __fish_spec.color.echo.failure "$failure_template"
+    return 1
+  end
+  if test "$FISH_SPEC_VERBOSE" = 1
+    eval __fish_spec.color.echo.success "$success_template"
+  end
+end
+
+function assert
+  set -l first "$argv"
+  __fish_spec_assert_generic \
+    $first unused \
+    'Assertion \"test $first\" passed!' \
+    'Assertion failed: \"test $first\" evaluated to false.' \
+    "test '$argv'"
+    return $status
+end
+
+function assert_equal -a first second
+  __fish_spec_assert_generic \
+    $first $second \
+    'Assertion \"$first\" == \"$second\" passed!' \
+    'Assertion failed: Expected \"$first\", but got \"$second\".' \
+    'test "$first" = "$second"'
+end
+
+function assert_not_equal -a first second
+  __fish_spec_assert_generic \
+    $first $second \
+    'Assertion \"$first\" != \"$second\" passed!' \
+    'Assertion failed: Expected \"$second\" to be different from \"$expecte\".' \
+    'test "$first" != "$second"'
+end
+
+function assert_exit_code -a expected_status
+  __fish_spec_assert_generic \
+    $expected_status $status \
+    'Assertion exit code $second == $first passed!' \
+    'Assertion failed: Expected exit code $first, but got $second.' \
+    'test "$first" -eq "$second"'
+end
+
+function assert_ok
+  assert_exit_code 0
+end
+
+function assert_true -a condition
+  __fish_spec_assert_generic \
+    $condition 0 \
+    'Assertion \"$first\" is true passed!' \
+    'Assertion failed: Expected true, but got \"$first\".' \
+    'test $first'
+end
+
+function assert_false -a condition
+  __fish_spec_assert_generic \
+    $condition 0 \
+    'Assertion \"$first\" is false passed!' \
+    'Assertion failed: Expected false, but got \"$first\".' \
+    'test ! $first'
+end
+
+function assert_match -a pattern string
+  __fish_spec_assert_generic \
+    $pattern $string \
+    'Assertion string \"$string\" matches pattern \"$pattern\" passed!' \
+    'Assertion failed: string \"$second\" does not match pattern \"$first\".' \
+    'string match -qr $first $second'
+end
+
+
+function assert_match -a pattern string
+  __fish_spec_assert_generic \
+    $pattern $string \
+    'Assertion string \"$string\" does not match pattern \"$pattern\" passed!' \
+    'Assertion failed: string \"$second\" does not match pattern \"$first\".' \
+    'not string match -qr $first $second'
+end
+
+function assert_file_exists -a file
+  __fish_spec_assert_generic \
+    $file unused \
+    'Assertion file \"$first\" exists passed!' \
+    'Assertion failed: File \"$first\" does not exist.' \
+    'test -f $first'
+end
+
+function assert_file_does_not_exist -a file
+  __fish_spec_assert_generic \
+    $file unused \
+    'Assertion file \"$first\" does not exist passed!' \
+    'Assertion failed: File \"$first\" exists.' \
+    'not test -f $first'
+end
+
+function assert_directory_exists -a dir
+  __fish_spec_assert_generic \
+    $dir unused \
+    'Assertion directory \"$first\" exists passed!' \
+    'Assertion failed: Directory \"$first\" does not exist.' \
+    'test -d $dir'
+end
+
+function assert_directory_does_not_exist -a file
+  __fish_spec_assert_generic \
+    $file unused \
+    'Assertion directory \"$first\" does not exist passed!' \
+    'Assertion failed: Directory \"$first\" exists.' \
+    'not test -d $first'
+end
+
+function assert_file_empty -a file
+  __fish_spec_assert_generic \
+    $file unused \
+    'Assertion file $first is empty passed!' \
+    'Assertion failed: File \"$first\" is not empty.' \
+    'not test -s $file'
+end
+
+function assert_file_contains -a file content
+  __fish_spec_assert_generic \
+    $file $content \
+    'Assertion $first contains \"$second\" passed!' \
+    'Assertion failed: File \"$first\" does not contain \"$second\".' \
+    'grep -q "$second" $first'
+end
+
+function assert_file_contains_regex -a file pattern
+  __fish_spec_assert_generic \
+    $file $pattern \
+    'Assertion $first content matches regex \"$second\" passed!' \
+    'Assertion failed: File \"$first\" does not contain a string matching the pattern \"$second\".' \
+    'grep -qE "$second" $first'
+end
+
+function assert_not_file_contains -a file content
+  __fish_spec_assert_generic \
+    $file $content \
+    'Assertion $first does not contain \"$content\" passed!' \
+    'Assertion failed: File \"$first\" contains \"$second\".' \
+    'not grep -q "$second" $first'
+end
+
+function assert_not_file_contains_regex -a file pattern
+  __fish_spec_assert_generic \
+    $file $pattern \
+    'Assertion $first content does not match regex \"$second\" passed!' \
+    'Assertion failed: File \"$first\" contains a string matching the pattern \"$second\".' \
+    'not grep -qE "$second" $first'
+end
+
+function assert_in_array -a value
+  set -g __fish_spec_assertion_array $argv[2..-1]
+  __fish_spec_assert_generic \
+    $value "$__fish_spec_assertion_array" \
+    'Assertion \"$first\" in [$second] passed!' \
+    'Assertion failed: Value \"$first\" is not in the array [$second].' \
+    'contains -- $first $__fish_spec_assertion_array'
+  set result $status
+  set -e __fish_spec_assertion_array
+  return $result
+end
+
+function assert_not_in_array -a value
+  set -g __fish_spec_assertion_array $argv[2..-1]
+  __fish_spec_assert_generic \
+    $value "$__fish_spec_assertion_array" \
+    'Assertion \"$first\" not in [$second] passed!' \
+    'Assertion failed: Value \"$first\" is in the array [$second].' \
+    'not contains -- $first $__fish_spec_assertion_array'
+  set result $status
+  set -e __fish_spec_assertion_array
+  return $result
+end

pkg/fish-spec/framework/color.fish

@@ -0,0 +1,39 @@
+function __fish_spec.color.echo.success
+  set_color -o green
+  echo $argv
+  set_color normal
+end
+
+function __fish_spec.color.echo.failure
+  set_color -o red
+  echo $argv
+  set_color normal
+end
+
+function __fish_spec.color.echo.mixed
+  set_color -o yellow
+  echo $argv
+  set_color normal
+end
+
+function __fish_spec.color.echo.info
+  set_color -o cyan
+  echo $argv
+  set_color normal
+end
+
+function __fish_spec.color.echo-n.info
+  echo -n (set_color -o cyan)$argv(set_color normal)
+end
+
+function __fish_spec.color.echo.autocolor -a total failed
+  if test $total -eq $failed -a $total -eq 0
+    __fish_spec.color.echo.success $argv[3..-1]
+  else if test $total -eq $failed
+    __fish_spec.color.echo.failure $argv[3..-1]
+  else if test $failed -gt 0
+    __fish_spec.color.echo.mixed $argv[3..-1]
+  else
+    __fish_spec.color.echo.success $argv[3..-1]
+  end
+end

pkg/fish-spec/functions/assert.error_message.fish

@@ -1,31 +0,0 @@
-function assert.error_message
-  set -l number_of_arguments (count $argv)
-
-  switch $argv[1]
-    case !
-      switch $number_of_arguments
-        case 3
-          set operator (assert.expand_operator $argv[2])
-          set actual $argv[3]
-          echo "Expected result to not be $operator but it was $actual"
-        case 4
-          set expected $argv[2]
-          set operator "not" (assert.expand_operator $argv[3])
-          set actual   $argv[4]
-          echo "Expected result to $operator $expected but it was $actual"
-        case \*
-          return 1
-        end
-    case \-\*
-      test $number_of_arguments != 2; and return 1
-      set operator (assert.expand_operator $argv[1])
-      set actual $argv[2]
-      echo "Expected result to be $operator but it was $actual"
-    case \*
-      test $number_of_arguments != 3; and return 1
-      set expected $argv[1]
-      set operator (assert.expand_operator $argv[2])
-      set actual   $argv[3]
-      echo "Expected result to $operator $expected but it was $actual"
-  end
-end

pkg/fish-spec/functions/assert.expand_operator.fish

@@ -1,10 +0,0 @@
-function assert.expand_operator -a operator
-  switch $operator
-    case =
-      echo equals
-    case \-z
-      echo empty
-    case \*
-      echo $operator
-  end
-end

pkg/fish-spec/functions/assert.fish

@@ -1,9 +0,0 @@
-function assert --wraps test
-  if builtin test $argv
-    emit assertion_success
-  else
-    set -l assertion_status $status
-    emit assertion_error (assert.error_message $argv)
-    return $assertion_status
-  end
-end

pkg/fish-spec/functions/fish-spec.fish

@@ -1,67 +1,103 @@
 function fish-spec
+  # set up fish-spec
   set -g __fish_spec_dir (dirname (dirname (status -f)))
-
-  # Source formatter
-  source $__fish_spec_dir/basic_formatter.fish
-
-  # Reset internal variables
-  set -e __any_spec_failed
-
-  # Load each spec file
-  for spec_file in spec/*_spec.fish
-    source $spec_file
+  for file in $__fish_spec_dir/framework/*.fish
+  source $file
   end
 
-  # Load helper file
-  source spec/helper.fish 2> /dev/null
+  # reset global assertion counters
+  set -g __fish_spec_failed_assertions 0
+  set -g __fish_spec_total_assertions 0
 
-  emit all_specs_init
-
-  # Run all specs
-  __fish-spec.run_all_specs
-
-  emit all_specs_finished
-
-  not set -q __any_spec_failed
-end
-
-function __fish-spec.run_all_specs
-  for suite in (functions -n | grep describe_)
-    __fish-spec.run_suite $suite
-    functions -e $suite
-  end
-end
-
-function __fish-spec.run_suite -a suite_name
-  # This gets the list of specs that were defined on the test suite by
-  # comparing the functions names before and after the evaluation of the test suite.
-  set -l specs (begin
-    functions -n | grep it_
-    eval $suite_name >/dev/null
-    functions -n | grep it_
-  end | sort | uniq -u)
-
-  functions -q before_all; and before_all
-
-  for spec in $specs
-    emit spec_init $spec
-    functions -q before_each; and before_each
-    eval $spec
-    functions -q after_each; and after_each
-    emit spec_finished $spec
-  end
-
-  functions -q after_all; and after_all
-
-  functions -e before_all before_each after_each after_all
-end
-
-function __fish-spec.current_time
-  if test (uname) = 'Darwin'
-    set filename 'epoch.osx'
+  if test "$argv" = ""
+    set test_files spec/*_spec.fish
   else
-    set filename 'epoch.linux'
+    set test_files $argv
   end
 
-  eval $__fish_spec_dir/utils/$filename
+  for test_file in $test_files
+    __fish_spec_run_tests_in_file $test_file
+  end
+
+  # Global summary
+  echo
+  __fish_spec.color.echo.autocolor $__fish_spec_total_assertions $__fish_spec_failed_assertions "Test complete: $__fish_spec_total_assertions assertions run, $__fish_spec_failed_assertions failed."
+  if test $__fish_spec_failed_assertions -gt 0
+    return 1
+  end
+end
+
+function __fish_spec_run_tests_in_file -a test_file
+  # reset per file assertion counters
+  set -g __fish_spec_failed_assertions_in_file 0
+  set -g __fish_spec_total_assertions_in_file 0
+  set -g __fish_spec_last_assertion_failed no
+
+  __fish_spec.color.echo.info "Running tests in $test_file..."
+  source $test_file
+
+  for suite in (functions | string match -r '^describe_.*')
+    __fish_spec_run_tests_in_suite $suite
+  end
+
+  functions -e (functions | string match -r '^(describe_.*)$')
+
+  # File-level summary
+  echo
+  __fish_spec.color.echo.autocolor $__fish_spec_total_assertions_in_file $__fish_spec_failed_assertions_in_file "Summary for $test_file: $__fish_spec_total_assertions_in_file assertions, $__fish_spec_failed_assertions_in_file failed."
+  echo
+end
+
+function __fish_spec_run_tests_in_suite -a suite
+  __fish_spec.color.echo.info (string replace 'describe_' 'DESCRIBE ' $suite | string replace '_' ' ')
+  $suite
+
+  if functions --query before_all
+    before_all
+  end
+
+  for test_func in (functions | string match -r '^it_.*' | sort)
+    __fish_spec_run_test_function $test_func
+  end
+
+  set __fish_spec_failed_assertions (math $__fish_spec_failed_assertions + $__fish_spec_failed_assertions_in_file)
+  set __fish_spec_total_assertions (math $__fish_spec_total_assertions + $__fish_spec_total_assertions_in_file)
+
+  if functions --query after_all
+    after_all
+  end
+
+  # Cleanup describe-scoped functions
+  functions -e (functions | string match -r '^(before_all|after_all|before_each|after_each|it_.*)$')
+end
+
+function __fish_spec_run_test_function -a test_func
+  set test_func_human_readable (string replace 'it_' 'IT ' $test_func | string replace -a '_' ' ')
+  __fish_spec.color.echo-n.info "⏳ $test_func_human_readable"
+
+  if functions --query before_each
+    set -l before_each_output (before_each 2>&1 | string collect)
+  end
+
+  set -l test_func_output ($test_func 2>&1 | string collect)
+  set result $status
+
+  if functions --query after_each
+    set -l before_each_output (before_each 2>&1 | string collect)
+  end
+
+  if test $__fish_spec_last_assertion_failed = no
+    __fish_spec.color.echo.success \r"✅ $test_func_human_readable passed!"
+    if test "$FISH_SPEC_VERBOSE" = 1
+      test -n "$before_each_output" && echo $before_each_output
+      test -n "$test_func_output" && echo $test_func_output
+      test -n "$after_each_output" && echo $after_each_output
+    end
+  else
+    __fish_spec.color.echo.failure \r"❌ $test_func_human_readable failed."
+    test -n "$before_each_output" && echo $before_each_output
+    test -n "$test_func_output" && echo $test_func_output
+    test -n "$after_each_output" && echo $after_each_output
+    set __fish_spec_last_assertion_failed no
+  end
 end

pkg/fish-spec/spec/assert_error_message_spec.fish

@@ -1,54 +0,0 @@
-function describe_assert_error_message
-  function before_each
-    set -g __current_spec_quiet
-  end
-
-  function after_each
-    set -e __current_spec_quiet
-  end
-
-  function it_has_no_output_when_the_test_succeeds
-    assert 1 = 1
-
-    # Reset test status
-    set -e __current_spec_status
-
-    assert -z "$__current_spec_output"
-  end
-
-  function it_supports_unary_operators
-    assert -z "abc"
-
-    # Reset test status
-    set -e __current_spec_status
-
-    assert 'Expected result to be empty but it was abc' = "$__current_spec_output"
-  end
-
-  function it_supports_binary_operators
-    assert 1 = 2
-
-    # Reset test status
-    set -e __current_spec_status
-
-    assert 'Expected result to equals 1 but it was 2' = "$__current_spec_output"
-  end
-
-  function it_supports_inversion_on_unary_operators
-    assert ! -z ""
-
-    # Reset test status
-    set -e __current_spec_status
-
-    assert 'Expected result to not be empty but it was ' = "$__current_spec_output"
-  end
-
-  function it_supports_inversion_on_binary_operators
-    assert ! 1 = 1
-
-    # Reset test status
-    set -e __current_spec_status
-
-    assert 'Expected result to not equals 1 but it was 1' = "$__current_spec_output"
-  end
-end

pkg/fish-spec/spec/results_spec.fish

@@ -2,38 +2,30 @@ function describe_results
   function it_succeeds_when_single_assertion_succeeds
     assert 1 = 1
 
-    assert success = "$__current_spec_status"
+    assert 0 = $status
   end
 
   function it_succeeds_when_multiple_assertion_succeeds
     assert 1 = 1
     assert 2 = 2
-
-    assert success = "$__current_spec_status"
   end
 
   function it_fails_when_single_assertion_fails
-    set -g __fish_spec_output "quiet"
-
+    set previous_assertion_counter $__fish_spec_failed_assertions_in_file
     assert 1 = 2
-    set -l spec_status $__current_spec_status
-
-    # Reset internals
-    set -e __current_spec_status
-
-    assert error = "$spec_status"
+    assert_exit_code 1
+    echo decrement failed assertion counter so tests pass as expected
+    set __fish_spec_failed_assertions_in_file (math $__fish_spec_failed_assertions_in_file - 1)
+    assert_equal $previous_assertion_counter $__fish_spec_failed_assertions_in_file
   end
 
   function it_fails_when_one_of_the_assertions_fails
-    set -g __fish_spec_output "quiet"
-
+    set previous_assertion_counter $__fish_spec_failed_assertions_in_file
     assert 1 = 2
+    assert_exit_code 1
     assert 2 = 2
-    set -l spec_status $__current_spec_status
-
-    # Reset internals
-    set -e __current_spec_status
-
-    assert error = "$spec_status"
+    echo decrement failed assertion counter so tests pass as expected
+    set __fish_spec_failed_assertions_in_file (math $__fish_spec_failed_assertions_in_file - 1)
+    assert_equal $previous_assertion_counter $__fish_spec_failed_assertions_in_file
   end
 end

pkg/fish-spec/utils/epoch.c

@@ -1,10 +0,0 @@
-#include <stdio.h>
-#include <sys/time.h>
-
-int main(int argc, char** argv) {
-  struct timeval time_struct;
-  gettimeofday(&time_struct, 0);
-  printf("%lld", (time_struct.tv_sec * 1000ll) + (time_struct.tv_usec / 1000ll));
-
-  return 0;
-}

pkg/omf/spec/basic_spec.fish

@@ -3,6 +3,10 @@ function describe_basic_tests
     set -gx CI WORKAROUND
   end
 
+  function before_all
+    set -e CI
+  end
+
   function it_has_a_help_command
     set -l output (omf help)
     echo $output | grep -Eq "cd.+Change to root or package directory"

pkg/omf/spec/omf_list_spec.fish

@@ -3,6 +3,10 @@ function describe_omf_list_tests
     set -gx CI WORKAROUND
   end
 
+  function before_all
+    set -e CI
+  end
+
   function it_can_list_plugins
     set -l list_output (omf list -p)
     assert 0 = $status

pkg/omf/spec/omf_packages_spec.fish

@@ -3,6 +3,10 @@ function describe_omf_packages_tests
     set -gx CI WORKAROUND
   end
 
+  function before_all
+    set -e CI
+  end
+
   function it_can_extract_name_from_name
     set -l output (omf.packages.name foo)
     assert 0 = $status