From 321c15407d285c0b7d43276c112c9542ac47bdfd Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 29 Nov 2022 03:14:24 -0700 Subject: [PATCH 01/67] Add utility to test specs individually --- .gitlab-ci.yml | 5 +++++ util/test-all-individually.sh | 5 +++++ 2 files changed, 10 insertions(+) create mode 100755 util/test-all-individually.sh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d627d27..438c34f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -51,6 +51,11 @@ spec mocks: script: - crystal spec --error-on-warnings --junit_output=. spec/spectator/mocks/ +spec individual: + extends: spec + script: + - util/test-all-individually.sh + format: script: - shards diff --git a/util/test-all-individually.sh b/util/test-all-individually.sh new file mode 100755 index 0000000..2984a03 --- /dev/null +++ b/util/test-all-individually.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash +set -e + +find spec/ -type f -name \*_spec.cr -print0 | \ + xargs -0 -n1 time crystal spec --error-on-warnings -v From 2d6c8844d47bc8d2b2ac47289711700e545a717f Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 29 Nov 2022 03:33:42 -0700 Subject: [PATCH 02/67] Remove `time` --- util/test-all-individually.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/test-all-individually.sh b/util/test-all-individually.sh index 2984a03..97bdd36 100755 --- a/util/test-all-individually.sh +++ b/util/test-all-individually.sh @@ -2,4 +2,4 @@ set -e find spec/ -type f -name \*_spec.cr -print0 | \ - xargs -0 -n1 time crystal spec --error-on-warnings -v + xargs -0 -n1 crystal spec --error-on-warnings -v From a585ef099656c96fd62ca70341f9e414387bf875 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 29 Nov 2022 20:28:15 -0700 Subject: [PATCH 03/67] Simplify string (inspect) representation These types make heavy use of generics and combined types. Instantiating string representation methods for all possibilities is unecesssary and slows down compilation. --- src/spectator/mocks/abstract_arguments.cr | 5 +++++ src/spectator/mocks/double.cr | 13 +++++++++++++ src/spectator/mocks/method_call.cr | 8 +++++++- 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/spectator/mocks/abstract_arguments.cr b/src/spectator/mocks/abstract_arguments.cr index 810d2fe..b03f161 100644 --- a/src/spectator/mocks/abstract_arguments.cr +++ b/src/spectator/mocks/abstract_arguments.cr @@ -1,6 +1,11 @@ module Spectator # Untyped arguments to a method call (message). abstract class AbstractArguments + # Use the string representation to avoid over complicating debug output. + def inspect(io : IO) : Nil + to_s(io) + end + # Utility method for comparing two named tuples ignoring order. private def compare_named_tuples(a : NamedTuple, b : NamedTuple) a.each do |k, v1| diff --git a/src/spectator/mocks/double.cr b/src/spectator/mocks/double.cr index 8143ba0..ad7d327 100644 --- a/src/spectator/mocks/double.cr +++ b/src/spectator/mocks/double.cr @@ -101,6 +101,19 @@ module Spectator io << _spectator_stubbed_name end + # :ditto: + def inspect(io : IO) : Nil + {% if anno = @type.annotation(::Spectator::StubbedName) %} + io << "#' + end + # Defines a stub to change the behavior of a method in this double. # # NOTE: Defining a stub for a method not defined in the double's type has no effect. diff --git a/src/spectator/mocks/method_call.cr b/src/spectator/mocks/method_call.cr index b8e973e..9c5fd01 100644 --- a/src/spectator/mocks/method_call.cr +++ b/src/spectator/mocks/method_call.cr @@ -30,7 +30,13 @@ module Spectator # Constructs a string containing the method name and arguments. def to_s(io : IO) : Nil - io << '#' << method << arguments + io << '#' << method + arguments.inspect(io) + end + + # :ditto: + def inspect(io : IO) : Nil + to_s(io) end end end From df10c8e75bfd583dacdec62f18f06daeaea36cd2 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 29 Nov 2022 20:29:36 -0700 Subject: [PATCH 04/67] Prevent multiple redefinitions of the same method --- src/spectator/mocks/stubbable.cr | 100 ++++++++++++++++--------------- 1 file changed, 51 insertions(+), 49 deletions(-) diff --git a/src/spectator/mocks/stubbable.cr b/src/spectator/mocks/stubbable.cr index cd7c407..ffb52f3 100644 --- a/src/spectator/mocks/stubbable.cr +++ b/src/spectator/mocks/stubbable.cr @@ -338,64 +338,66 @@ module Spectator # Redefines all methods and ones inherited from its parents and mixins to support stubs. private macro stub_type(type_name = @type) {% type = type_name.resolve - # Reverse order of ancestors (there's currently no reverse method for ArrayLiteral). - count = type.ancestors.size - ancestors = type.ancestors.map_with_index { |_, i| type.ancestors[count - i - 1] } %} - {% for ancestor in ancestors %} - {% for method in ancestor.methods.reject do |meth| - meth.name.starts_with?("_spectator") || - ::Spectator::DSL::RESERVED_KEYWORDS.includes?(meth.name.symbolize) - end %} - {{(method.abstract? ? :abstract_stub : :default_stub).id}} {{method.visibility.id if method.visibility != :public}} def {{"#{method.receiver}.".id if method.receiver}}{{method.name}}( - {% for arg, i in method.args %}{% if i == method.splat_index %}*{% end %}{{arg}}, {% end %} - {% if method.double_splat %}**{{method.double_splat}}, {% end %} - {% if method.block_arg %}&{{method.block_arg}}{% elsif method.accepts_block? %}&{% end %} - ){% if method.return_type %} : {{method.return_type}}{% end %}{% if !method.free_vars.empty? %} forall {{method.free_vars.splat}}{% end %} - super{% if method.accepts_block? %} { |*%yargs| yield *%yargs }{% end %} - end - {% end %} + definitions = [] of Nil + scope = (type == @type ? :previous_def : :super).id - {% for method in ancestor.class.methods.reject do |meth| - meth.name.starts_with?("_spectator") || - ::Spectator::DSL::RESERVED_KEYWORDS.includes?(meth.name.symbolize) - end %} - default_stub {{method.visibility.id if method.visibility != :public}} def self.{{method.name}}( - {% for arg, i in method.args %}{% if i == method.splat_index %}*{% end %}{{arg}}, {% end %} - {% if method.double_splat %}**{{method.double_splat}}, {% end %} - {% if method.block_arg %}&{{method.block_arg}}{% elsif method.accepts_block? %}&{% end %} - ){% if method.return_type %} : {{method.return_type}}{% end %}{% if !method.free_vars.empty? %} forall {{method.free_vars.splat}}{% end %} - super{% if method.accepts_block? %} { |*%yargs| yield *%yargs }{% end %} - end - {% end %} - {% end %} + # Add entries for methods in the target type and its class type. + [[:self.id, type.class], [nil, type]].each do |(receiver, t)| + t.methods.each do |method| + definitions << { + type: t, + method: method, + scope: scope, + receiver: receiver, + } + end + end - {% for method in type.methods.reject do |meth| - meth.name.starts_with?("_spectator") || - ::Spectator::DSL::RESERVED_KEYWORDS.includes?(meth.name.symbolize) - end %} - {{(method.abstract? ? :"abstract_stub abstract" : :default_stub).id}} {{method.visibility.id if method.visibility != :public}} def {{"#{method.receiver}.".id if method.receiver}}{{method.name}}( + # Iterate through all ancestors and add their methods. + type.ancestors.each do |ancestor| + [[:self.id, ancestor.class], [nil, ancestor]].each do |(receiver, t)| + t.methods.each do |method| + # Skip methods already found to prevent redefining them multiple times. + unless definitions.any? do |d| + m = d[:method] + m.name == method.name && + m.args == method.args && + m.splat_index == method.splat_index && + m.double_splat == method.double_splat && + m.block_arg == method.block_arg + end + definitions << { + type: t, + method: method, + scope: :super.id, + receiver: receiver, + } + end + end + end + end + + definitions = definitions.reject do |definition| + name = definition[:method].name + name.starts_with?("_spectator") || ::Spectator::DSL::RESERVED_KEYWORDS.includes?(name.symbolize) + end %} + + {% for definition in definitions %} + {% original_type = definition[:type] + method = definition[:method] + scope = definition[:scope] + receiver = definition[:receiver] %} + # Redefinition of {{type}}{{(receiver ? "." : "#").id}}{{method.name}} + {{(method.abstract? ? "abstract_stub abstract" : "default_stub").id}} {{method.visibility.id if method.visibility != :public}} def {{"#{receiver}.".id if receiver}}{{method.name}}( {% for arg, i in method.args %}{% if i == method.splat_index %}*{% end %}{{arg}}, {% end %} {% if method.double_splat %}**{{method.double_splat}}, {% end %} {% if method.block_arg %}&{{method.block_arg}}{% elsif method.accepts_block? %}&{% end %} ){% if method.return_type %} : {{method.return_type}}{% end %}{% if !method.free_vars.empty? %} forall {{method.free_vars.splat}}{% end %} - {% unless method.abstract? %} - {% if type == @type %}previous_def{% else %}super{% end %}{% if method.accepts_block? %} { |*%yargs| yield *%yargs }{% end %} + {% unless method.abstract? %} + {{scope}}{% if method.accepts_block? %} { |*%yargs| yield *%yargs }{% end %} end {% end %} {% end %} - - {% for method in type.class.methods.reject do |meth| - meth.name.starts_with?("_spectator") || - ::Spectator::DSL::RESERVED_KEYWORDS.includes?(meth.name.symbolize) - end %} - default_stub {{method.visibility.id if method.visibility != :public}} def self.{{method.name}}( - {% for arg, i in method.args %}{% if i == method.splat_index %}*{% end %}{{arg}}, {% end %} - {% if method.double_splat %}**{{method.double_splat}}, {% end %} - {% if method.block_arg %}&{{method.block_arg}}{% elsif method.accepts_block? %}&{% end %} - ){% if method.return_type %} : {{method.return_type}}{% end %}{% if !method.free_vars.empty? %} forall {{method.free_vars.splat}}{% end %} - {% if type == @type %}previous_def{% else %}super{% end %}{% if method.accepts_block? %} { |*%yargs| yield *%yargs }{% end %} - end - {% end %} end # Utility macro for casting a stub (and its return value) to the correct type. From 5f499336acf2b20c47d7e63db17f069e0d5520ac Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 29 Nov 2022 20:30:42 -0700 Subject: [PATCH 05/67] Remove individual spec runs from CI --- .gitlab-ci.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 438c34f..d627d27 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -51,11 +51,6 @@ spec mocks: script: - crystal spec --error-on-warnings --junit_output=. spec/spectator/mocks/ -spec individual: - extends: spec - script: - - util/test-all-individually.sh - format: script: - shards From 1f98bf9ff122e6e767418312b6a3e27d68ab3a50 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 29 Nov 2022 20:32:45 -0700 Subject: [PATCH 06/67] Update CHANGELOG --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88b37de..c13bd55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] +### Changed +- Simplify string representation of mock-related types. +- Remove unnecessary redefinitions of methods when adding stub functionality to a type. + ## [0.11.4] ### Added - Add support for using named (keyword) arguments in place of positional arguments in stubs. [#47](https://github.com/icy-arctic-fox/spectator/issues/47) From a967dce241daf071c6b75cd045d906ece6821ee6 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 29 Nov 2022 21:24:31 -0700 Subject: [PATCH 07/67] Adjust double string representation to_s and inspect (with variants) are no longer "original implementation." --- spec/spectator/mocks/double_spec.cr | 70 ++++++++++++++++++++++-- spec/spectator/mocks/lazy_double_spec.cr | 66 +++++++++++++++++++++- spec/spectator/mocks/null_double_spec.cr | 67 ++++++++++++++++++++++- src/spectator/mocks/double.cr | 30 +++++----- src/spectator/mocks/lazy_double.cr | 6 +- src/spectator/mocks/null_double.cr | 8 +-- 6 files changed, 215 insertions(+), 32 deletions(-) diff --git a/spec/spectator/mocks/double_spec.cr b/spec/spectator/mocks/double_spec.cr index 1b1abc9..41bfad2 100644 --- a/spec/spectator/mocks/double_spec.cr +++ b/spec/spectator/mocks/double_spec.cr @@ -212,14 +212,10 @@ Spectator.describe Spectator::Double do expect(dbl.hash).to be_a(UInt64) expect(dbl.in?([42])).to be_false expect(dbl.in?(1, 2, 3)).to be_false - expect(dbl.inspect).to contain("EmptyDouble") expect(dbl.itself).to be(dbl) expect(dbl.not_nil!).to be(dbl) - expect(dbl.pretty_inspect).to contain("EmptyDouble") expect(dbl.pretty_print(pp)).to be_nil expect(dbl.tap { nil }).to be(dbl) - expect(dbl.to_s).to contain("EmptyDouble") - expect(dbl.to_s(io)).to be_nil expect(dbl.try { nil }).to be_nil expect(dbl.object_id).to be_a(UInt64) expect(dbl.same?(dbl)).to be_true @@ -469,7 +465,7 @@ Spectator.describe Spectator::Double do it "stores calls to non-stubbed methods" do expect { dbl.baz }.to raise_error(Spectator::UnexpectedMessage, /baz/) - expect(called_method_names(dbl)).to eq(%i[baz]) + expect(called_method_names(dbl)).to contain(:baz) end it "stores arguments for a call" do @@ -479,4 +475,68 @@ Spectator.describe Spectator::Double do expect(call.arguments).to eq(args) end end + + describe "#to_s" do + subject(string) { dbl.to_s } + + context "with a name" do + let(dbl) { FooBarDouble.new } + + it "indicates it's a double" do + expect(string).to contain("Double") + end + + it "contains the double name" do + expect(string).to contain("dbl-name") + end + end + + context "without a name" do + let(dbl) { EmptyDouble.new } + + it "indicates it's a double" do + expect(string).to contain("Double") + end + + it "contains \"Anonymous\"" do + expect(string).to contain("Anonymous") + end + end + end + + describe "#inspect" do + subject(string) { dbl.inspect } + + context "with a name" do + let(dbl) { FooBarDouble.new } + + it "indicates it's a double" do + expect(string).to contain("Double") + end + + it "contains the double name" do + expect(string).to contain("dbl-name") + end + + it "contains the object ID" do + expect(string).to contain(dbl.object_id.to_s(16)) + end + end + + context "without a name" do + let(dbl) { EmptyDouble.new } + + it "indicates it's a double" do + expect(string).to contain("Double") + end + + it "contains \"Anonymous\"" do + expect(string).to contain("Anonymous") + end + + it "contains the object ID" do + expect(string).to contain(dbl.object_id.to_s(16)) + end + end + end end diff --git a/spec/spectator/mocks/lazy_double_spec.cr b/spec/spectator/mocks/lazy_double_spec.cr index c3402b5..8ea5a5d 100644 --- a/spec/spectator/mocks/lazy_double_spec.cr +++ b/spec/spectator/mocks/lazy_double_spec.cr @@ -275,7 +275,7 @@ Spectator.describe Spectator::LazyDouble do it "stores calls to non-stubbed methods" do expect { dbl.baz }.to raise_error(Spectator::UnexpectedMessage, /baz/) - expect(called_method_names(dbl)).to eq(%i[baz]) + expect(called_method_names(dbl)).to contain(:baz) end it "stores arguments for a call" do @@ -285,4 +285,68 @@ Spectator.describe Spectator::LazyDouble do expect(call.arguments).to eq(args) end end + + describe "#to_s" do + subject(string) { dbl.to_s } + + context "with a name" do + let(dbl) { Spectator::LazyDouble.new("dbl-name") } + + it "indicates it's a double" do + expect(string).to contain("LazyDouble") + end + + it "contains the double name" do + expect(string).to contain("dbl-name") + end + end + + context "without a name" do + let(dbl) { Spectator::LazyDouble.new } + + it "contains the double type" do + expect(string).to contain("LazyDouble") + end + + it "contains \"Anonymous\"" do + expect(string).to contain("Anonymous") + end + end + end + + describe "#inspect" do + subject(string) { dbl.inspect } + + context "with a name" do + let(dbl) { Spectator::LazyDouble.new("dbl-name") } + + it "contains the double type" do + expect(string).to contain("LazyDouble") + end + + it "contains the double name" do + expect(string).to contain("dbl-name") + end + + it "contains the object ID" do + expect(string).to contain(dbl.object_id.to_s(16)) + end + end + + context "without a name" do + let(dbl) { Spectator::LazyDouble.new } + + it "contains the double type" do + expect(string).to contain("LazyDouble") + end + + it "contains \"Anonymous\"" do + expect(string).to contain("Anonymous") + end + + it "contains the object ID" do + expect(string).to contain(dbl.object_id.to_s(16)) + end + end + end end diff --git a/spec/spectator/mocks/null_double_spec.cr b/spec/spectator/mocks/null_double_spec.cr index 4a8ef3a..ad87ea9 100644 --- a/spec/spectator/mocks/null_double_spec.cr +++ b/spec/spectator/mocks/null_double_spec.cr @@ -186,12 +186,9 @@ Spectator.describe Spectator::NullDouble do expect(dbl.hash).to be_a(UInt64) expect(dbl.in?([42])).to be_false expect(dbl.in?(1, 2, 3)).to be_false - expect(dbl.inspect).to contain("EmptyDouble") expect(dbl.itself).to be(dbl) expect(dbl.not_nil!).to be(dbl) - expect(dbl.pretty_inspect).to contain("EmptyDouble") expect(dbl.tap { nil }).to be(dbl) - expect(dbl.to_s).to contain("EmptyDouble") expect(dbl.try { nil }).to be_nil expect(dbl.object_id).to be_a(UInt64) expect(dbl.same?(dbl)).to be_true @@ -439,4 +436,68 @@ Spectator.describe Spectator::NullDouble do expect(call.arguments).to eq(args) end end + + describe "#to_s" do + subject(string) { dbl.to_s } + + context "with a name" do + let(dbl) { FooBarDouble.new } + + it "indicates it's a double" do + expect(string).to contain("NullDouble") + end + + it "contains the double name" do + expect(string).to contain("dbl-name") + end + end + + context "without a name" do + let(dbl) { EmptyDouble.new } + + it "contains the double type" do + expect(string).to contain("NullDouble") + end + + it "contains \"Anonymous\"" do + expect(string).to contain("Anonymous") + end + end + end + + describe "#inspect" do + subject(string) { dbl.inspect } + + context "with a name" do + let(dbl) { FooBarDouble.new } + + it "contains the double type" do + expect(string).to contain("NullDouble") + end + + it "contains the double name" do + expect(string).to contain("dbl-name") + end + + it "contains the object ID" do + expect(string).to contain(dbl.object_id.to_s(16)) + end + end + + context "without a name" do + let(dbl) { EmptyDouble.new } + + it "contains the double type" do + expect(string).to contain("NullDouble") + end + + it "contains \"Anonymous\"" do + expect(string).to contain("Anonymous") + end + + it "contains the object ID" do + expect(string).to contain(dbl.object_id.to_s(16)) + end + end + end end diff --git a/src/spectator/mocks/double.cr b/src/spectator/mocks/double.cr index ad7d327..e5e43f4 100644 --- a/src/spectator/mocks/double.cr +++ b/src/spectator/mocks/double.cr @@ -98,16 +98,14 @@ module Spectator # Simplified string representation of a double. # Avoids displaying nested content and bloating method instantiation. def to_s(io : IO) : Nil - io << _spectator_stubbed_name + io << "#<" + {{@type.name(generic_args: false).stringify}} + " " + io << _spectator_stubbed_name << '>' end # :ditto: def inspect(io : IO) : Nil - {% if anno = @type.annotation(::Spectator::StubbedName) %} - io << "#" + {{(anno[0] || :Anonymous.id).stringify}} {% else %} - "#" + "Anonymous" {% end %} end private def self._spectator_stubbed_name : String {% if anno = @type.annotation(StubbedName) %} - "#" + {{(anno[0] || :Anonymous.id).stringify}} {% else %} - "#" + "Anonymous" {% end %} end @@ -188,7 +186,7 @@ module Spectator "Stubs are defined for #{call.method.inspect}, but none matched (no argument constraints met)." end - raise UnexpectedMessage.new("#{_spectator_stubbed_name} received unexpected message #{call}") + raise UnexpectedMessage.new("#{inspect} received unexpected message #{call}") end private def _spectator_abstract_stub_fallback(call : MethodCall, type) @@ -207,9 +205,9 @@ module Spectator call = ::Spectator::MethodCall.new({{call.name.symbolize}}, args) _spectator_record_call(call) - Log.trace { "#{_spectator_stubbed_name} got undefined method `#{call}{% if call.block %} { ... }{% end %}`" } + Log.trace { "#{inspect} got undefined method `#{call}{% if call.block %} { ... }{% end %}`" } - raise ::Spectator::UnexpectedMessage.new("#{_spectator_stubbed_name} received unexpected message #{call}") + raise ::Spectator::UnexpectedMessage.new("#{inspect} received unexpected message #{call}") nil # Necessary for compiler to infer return type as nil. Avoids runtime "can't execute ... `x` has no type errors". end end diff --git a/src/spectator/mocks/lazy_double.cr b/src/spectator/mocks/lazy_double.cr index 9a9257d..75fe30c 100644 --- a/src/spectator/mocks/lazy_double.cr +++ b/src/spectator/mocks/lazy_double.cr @@ -37,13 +37,13 @@ module Spectator # Returns the double's name formatted for user output. private def _spectator_stubbed_name : String - "#" + @name || "Anonymous" end private def _spectator_stub_fallback(call : MethodCall, &) if _spectator_stub_for_method?(call.method) Log.info { "Stubs are defined for #{call.method.inspect}, but none matched (no argument constraints met)." } - raise UnexpectedMessage.new("#{_spectator_stubbed_name} received unexpected message #{call}") + raise UnexpectedMessage.new("#{inspect} received unexpected message #{call}") else Log.trace { "Fallback for #{call} - call original" } yield @@ -57,7 +57,7 @@ module Spectator %call = ::Spectator::MethodCall.new({{call.name.symbolize}}, %args) _spectator_record_call(%call) - Log.trace { "#{_spectator_stubbed_name} got undefined method `#{%call}{% if call.block %} { ... }{% end %}`" } + Log.trace { "#{inspect} got undefined method `#{%call}{% if call.block %} { ... }{% end %}`" } # Attempt to find a stub that satisfies the method call and arguments. if %stub = _spectator_find_stub(%call) diff --git a/src/spectator/mocks/null_double.cr b/src/spectator/mocks/null_double.cr index 0ddd03d..587f4ab 100644 --- a/src/spectator/mocks/null_double.cr +++ b/src/spectator/mocks/null_double.cr @@ -26,7 +26,7 @@ module Spectator private def _spectator_abstract_stub_fallback(call : MethodCall) if _spectator_stub_for_method?(call.method) Log.info { "Stubs are defined for #{call.method.inspect}, but none matched (no argument constraints met)." } - raise UnexpectedMessage.new("#{_spectator_stubbed_name} received unexpected message #{call}") + raise UnexpectedMessage.new("#{inspect} received unexpected message #{call}") else Log.trace { "Fallback for #{call} - return self" } self @@ -42,9 +42,9 @@ module Spectator private def _spectator_abstract_stub_fallback(call : MethodCall, type) if _spectator_stub_for_method?(call.method) Log.info { "Stubs are defined for #{call.method.inspect}, but none matched (no argument constraints met)." } - raise UnexpectedMessage.new("#{_spectator_stubbed_name} received unexpected message #{call}") + raise UnexpectedMessage.new("#{inspect} received unexpected message #{call}") else - raise TypeCastError.new("#{_spectator_stubbed_name} received message #{call} and is attempting to return `self`, but returned type must be `#{type}`.") + raise TypeCastError.new("#{inspect} received message #{call} and is attempting to return `self`, but returned type must be `#{type}`.") end end @@ -56,7 +56,7 @@ module Spectator %call = ::Spectator::MethodCall.new({{call.name.symbolize}}, %args) _spectator_record_call(%call) - Log.trace { "#{_spectator_stubbed_name} got undefined method `#{%call}{% if call.block %} { ... }{% end %}`" } + Log.trace { "#{inspect} got undefined method `#{%call}{% if call.block %} { ... }{% end %}`" } self end From fbe877690d1e01ac887abe6022d859b728a305b8 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 29 Nov 2022 22:31:22 -0700 Subject: [PATCH 08/67] Adjust call argument matching Reenable test for https://github.com/icy-arctic-fox/spectator/issues/44 and https://github.com/icy-arctic-fox/spectator/issues/47 --- spec/issues/github_issue_44_spec.cr | 11 +++++++---- src/spectator/mocks/arguments.cr | 3 ++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/spec/issues/github_issue_44_spec.cr b/spec/issues/github_issue_44_spec.cr index d1b9716..da6cbf3 100644 --- a/spec/issues/github_issue_44_spec.cr +++ b/spec/issues/github_issue_44_spec.cr @@ -26,12 +26,15 @@ Spectator.describe "GitHub Issue #44" do # Original issue uses keyword arguments in place of positional arguments. context "keyword arguments in place of positional arguments" do before_each do - expect(Process).to receive(:run).with(command, shell: true, output: :pipe).and_raise(exception) + pipe = Process::Redirect::Pipe + expect(Process).to receive(:run).with(command, shell: true, output: pipe).and_raise(exception) end - it "must stub Process.run", skip: "Keyword arguments in place of positional arguments not supported with expect-receive" do - Process.run(command, shell: true, output: :pipe) do |_process| - end + it "must stub Process.run" do + expect do + Process.run(command, shell: true, output: :pipe) do |_process| + end + end.to raise_error(File::NotFoundError, "File not found") end end end diff --git a/src/spectator/mocks/arguments.cr b/src/spectator/mocks/arguments.cr index f9c6e19..fcb74f4 100644 --- a/src/spectator/mocks/arguments.cr +++ b/src/spectator/mocks/arguments.cr @@ -90,9 +90,10 @@ module Spectator i = 0 other.args.each do |k, v2| + break if i >= positional.size next if kwargs.has_key?(k) # Covered by named arguments. - v1 = positional.fetch(i) { return false } + v1 = positional[i] i += 1 return false unless v1 === v2 end From 275b217c6c1238d8fb0934b733e1919e883eb7b4 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 29 Nov 2022 23:22:42 -0700 Subject: [PATCH 09/67] Allow metadata to be stored as nil --- CHANGELOG.md | 1 + src/spectator/dsl/metadata.cr | 3 +++ src/spectator/example.cr | 11 +++++----- src/spectator/example_builder.cr | 4 ++-- src/spectator/example_group.cr | 2 +- src/spectator/example_group_builder.cr | 2 +- src/spectator/example_group_iteration.cr | 2 +- .../iterative_example_group_builder.cr | 2 +- src/spectator/node.cr | 20 ++++++++++++++----- src/spectator/pending_example_builder.cr | 2 +- src/spectator/spec_builder.cr | 8 ++++---- src/spectator/tag_node_filter.cr | 4 +++- src/spectator/test_context.cr | 4 ++-- 13 files changed, 41 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c13bd55..704bc9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Simplify string representation of mock-related types. - Remove unnecessary redefinitions of methods when adding stub functionality to a type. +- Allow metadata to be stored as nil to reduce overhead when tracking nodes without tags. ## [0.11.4] ### Added diff --git a/src/spectator/dsl/metadata.cr b/src/spectator/dsl/metadata.cr index 308bcbd..04092b9 100644 --- a/src/spectator/dsl/metadata.cr +++ b/src/spectator/dsl/metadata.cr @@ -6,6 +6,9 @@ module Spectator::DSL private macro _spectator_metadata(name, source, *tags, **metadata) private def self.{{name.id}} %metadata = {{source.id}}.dup + {% unless tags.empty? && metadata.empty? %} + %metadata ||= ::Spectator::Metadata.new + {% end %} {% for k in tags %} %metadata[{{k.id.symbolize}}] = nil {% end %} diff --git a/src/spectator/example.cr b/src/spectator/example.cr index 3625ded..67483e0 100644 --- a/src/spectator/example.cr +++ b/src/spectator/example.cr @@ -40,7 +40,7 @@ module Spectator # Note: The metadata will not be merged with the parent metadata. def initialize(@context : Context, @entrypoint : self ->, name : String? = nil, location : Location? = nil, - @group : ExampleGroup? = nil, metadata = Metadata.new) + @group : ExampleGroup? = nil, metadata = nil) super(name, location, metadata) # Ensure group is linked. @@ -58,7 +58,7 @@ module Spectator # Note: The metadata will not be merged with the parent metadata. def initialize(@context : Context, @entrypoint : self ->, @name_proc : Example -> String, location : Location? = nil, - @group : ExampleGroup? = nil, metadata = Metadata.new) + @group : ExampleGroup? = nil, metadata = nil) super(nil, location, metadata) # Ensure group is linked. @@ -75,7 +75,7 @@ module Spectator # A set of *metadata* can be used for filtering and modifying example behavior. # Note: The metadata will not be merged with the parent metadata. def initialize(name : String? = nil, location : Location? = nil, - @group : ExampleGroup? = nil, metadata = Metadata.new, &block : self ->) + @group : ExampleGroup? = nil, metadata = nil, &block : self ->) super(name, location, metadata) @context = NullContext.new @@ -93,9 +93,10 @@ module Spectator # A set of *metadata* can be used for filtering and modifying example behavior. # Note: The metadata will not be merged with the parent metadata. def self.pending(name : String? = nil, location : Location? = nil, - group : ExampleGroup? = nil, metadata = Metadata.new, reason = nil) + group : ExampleGroup? = nil, metadata = nil, reason = nil) # Add pending tag and reason if they don't exist. - metadata = metadata.merge({:pending => nil, :reason => reason}) { |_, v, _| v } + tags = {:pending => nil, :reason => reason} + metadata = metadata ? metadata.merge(tags) { |_, v, _| v } : tags new(name, location, group, metadata) { nil } end diff --git a/src/spectator/example_builder.cr b/src/spectator/example_builder.cr index bb640df..23398d2 100644 --- a/src/spectator/example_builder.cr +++ b/src/spectator/example_builder.cr @@ -15,7 +15,7 @@ module Spectator # The *entrypoint* indicates the proc used to invoke the test code in the example. # The *name*, *location*, and *metadata* will be applied to the `Example` produced by `#build`. def initialize(@context_builder : -> Context, @entrypoint : Example ->, - @name : String? = nil, @location : Location? = nil, @metadata : Metadata = Metadata.new) + @name : String? = nil, @location : Location? = nil, @metadata : Metadata? = nil) end # Creates the builder. @@ -24,7 +24,7 @@ module Spectator # The *name* is an interpolated string that runs in the context of the example. # *location*, and *metadata* will be applied to the `Example` produced by `#build`. def initialize(@context_builder : -> Context, @entrypoint : Example ->, - @name : Example -> String, @location : Location? = nil, @metadata : Metadata = Metadata.new) + @name : Example -> String, @location : Location? = nil, @metadata : Metadata? = nil) end # Constructs an example with previously defined attributes and context. diff --git a/src/spectator/example_group.cr b/src/spectator/example_group.cr index dc1fa57..277be3c 100644 --- a/src/spectator/example_group.cr +++ b/src/spectator/example_group.cr @@ -79,7 +79,7 @@ module Spectator # This group will be assigned to the parent *group* if it is provided. # A set of *metadata* can be used for filtering and modifying example behavior. def initialize(@name : Label = nil, @location : Location? = nil, - @group : ExampleGroup? = nil, @metadata : Metadata = Metadata.new) + @group : ExampleGroup? = nil, @metadata : Metadata? = nil) # Ensure group is linked. group << self if group end diff --git a/src/spectator/example_group_builder.cr b/src/spectator/example_group_builder.cr index 05c740f..207cb6e 100644 --- a/src/spectator/example_group_builder.cr +++ b/src/spectator/example_group_builder.cr @@ -28,7 +28,7 @@ module Spectator # Creates the builder. # Initially, the builder will have no children and no hooks. # The *name*, *location*, and *metadata* will be applied to the `ExampleGroup` produced by `#build`. - def initialize(@name : Label = nil, @location : Location? = nil, @metadata : Metadata = Metadata.new) + def initialize(@name : Label = nil, @location : Location? = nil, @metadata : Metadata? = nil) end # Constructs an example group with previously defined attributes, children, and hooks. diff --git a/src/spectator/example_group_iteration.cr b/src/spectator/example_group_iteration.cr index d6576d2..0d20a29 100644 --- a/src/spectator/example_group_iteration.cr +++ b/src/spectator/example_group_iteration.cr @@ -18,7 +18,7 @@ module Spectator # This group will be assigned to the parent *group* if it is provided. # A set of *metadata* can be used for filtering and modifying example behavior. def initialize(@item : T, name : Label = nil, location : Location? = nil, - group : ExampleGroup? = nil, metadata : Metadata = Metadata.new) + group : ExampleGroup? = nil, metadata : Metadata? = nil) super(name, location, group, metadata) end end diff --git a/src/spectator/iterative_example_group_builder.cr b/src/spectator/iterative_example_group_builder.cr index 39ad549..fe67c7a 100644 --- a/src/spectator/iterative_example_group_builder.cr +++ b/src/spectator/iterative_example_group_builder.cr @@ -15,7 +15,7 @@ module Spectator # The *collection* is the set of items to create sub-nodes for. # The *iterators* is a list of optional names given to items in the collection. def initialize(@collection : Enumerable(T), name : String? = nil, @iterators : Array(String) = [] of String, - location : Location? = nil, metadata : Metadata = Metadata.new) + location : Location? = nil, metadata : Metadata? = nil) super(name, location, metadata) end diff --git a/src/spectator/node.cr b/src/spectator/node.cr index 807c8df..6a5d068 100644 --- a/src/spectator/node.cr +++ b/src/spectator/node.cr @@ -30,14 +30,16 @@ module Spectator end # User-defined tags and values used for filtering and behavior modification. - getter metadata : Metadata + def metadata : Metadata + @metadata ||= Metadata.new + end # Creates the node. # The *name* describes the purpose of the node. # It can be a `Symbol` to describe a type. # The *location* tracks where the node exists in source code. # A set of *metadata* can be used for filtering and modifying example behavior. - def initialize(@name : Label = nil, @location : Location? = nil, @metadata : Metadata = Metadata.new) + def initialize(@name : Label = nil, @location : Location? = nil, @metadata : Metadata? = nil) end # Indicates whether the node has completed. @@ -46,17 +48,25 @@ module Spectator # Checks if the node has been marked as pending. # Pending items should be skipped during execution. def pending? - metadata.has_key?(:pending) || metadata.has_key?(:skip) + return false unless md = @metadata + + md.has_key?(:pending) || md.has_key?(:skip) end # Gets the reason the node has been marked as pending. def pending_reason - metadata[:pending]? || metadata[:skip]? || metadata[:reason]? || DEFAULT_PENDING_REASON + return DEFAULT_PENDING_REASON unless md = @metadata + + md[:pending]? || md[:skip]? || md[:reason]? || DEFAULT_PENDING_REASON end # Retrieves just the tag names applied to the node. def tags - Tags.new(metadata.keys) + if md = @metadata + Tags.new(md.keys) + else + Tags.new + end end # Non-nil name used to show the node name. diff --git a/src/spectator/pending_example_builder.cr b/src/spectator/pending_example_builder.cr index a1f0292..434efe5 100644 --- a/src/spectator/pending_example_builder.cr +++ b/src/spectator/pending_example_builder.cr @@ -11,7 +11,7 @@ module Spectator # The *name*, *location*, and *metadata* will be applied to the `Example` produced by `#build`. # A default *reason* can be given in case the user didn't provide one. def initialize(@name : String? = nil, @location : Location? = nil, - @metadata : Metadata = Metadata.new, @reason : String? = nil) + @metadata : Metadata? = nil, @reason : String? = nil) end # Constructs an example with previously defined attributes. diff --git a/src/spectator/spec_builder.cr b/src/spectator/spec_builder.cr index 265b41d..17b0284 100644 --- a/src/spectator/spec_builder.cr +++ b/src/spectator/spec_builder.cr @@ -60,7 +60,7 @@ module Spectator # # A set of *metadata* can be used for filtering and modifying example behavior. # For instance, adding a "pending" tag will mark tests as pending and skip execution. - def start_group(name, location = nil, metadata = Metadata.new) : Nil + def start_group(name, location = nil, metadata = nil) : Nil Log.trace { "Start group: #{name.inspect} @ #{location}; metadata: #{metadata}" } builder = ExampleGroupBuilder.new(name, location, metadata) @@ -86,7 +86,7 @@ module Spectator # # A set of *metadata* can be used for filtering and modifying example behavior. # For instance, adding a "pending" tag will mark tests as pending and skip execution. - def start_iterative_group(collection, name, iterator = nil, location = nil, metadata = Metadata.new) : Nil + def start_iterative_group(collection, name, iterator = nil, location = nil, metadata = nil) : Nil Log.trace { "Start iterative group: #{name} (#{typeof(collection)}) @ #{location}; metadata: #{metadata}" } builder = IterativeExampleGroupBuilder.new(collection, name, iterator, location, metadata) @@ -127,7 +127,7 @@ module Spectator # It will be yielded two arguments - the example created by this method, and the *context* argument. # The return value of the block is ignored. # It is expected that the test code runs when the block is called. - def add_example(name, location, context_builder, metadata = Metadata.new, &block : Example -> _) : Nil + def add_example(name, location, context_builder, metadata = nil, &block : Example -> _) : Nil Log.trace { "Add example: #{name} @ #{location}; metadata: #{metadata}" } current << ExampleBuilder.new(context_builder, block, name, location, metadata) end @@ -144,7 +144,7 @@ module Spectator # A set of *metadata* can be used for filtering and modifying example behavior. # For instance, adding a "pending" tag will mark the test as pending and skip execution. # A default *reason* can be given in case the user didn't provide one. - def add_pending_example(name, location, metadata = Metadata.new, reason = nil) : Nil + def add_pending_example(name, location, metadata = nil, reason = nil) : Nil Log.trace { "Add pending example: #{name} @ #{location}; metadata: #{metadata}" } current << PendingExampleBuilder.new(name, location, metadata, reason) end diff --git a/src/spectator/tag_node_filter.cr b/src/spectator/tag_node_filter.cr index d360712..0dedd59 100644 --- a/src/spectator/tag_node_filter.cr +++ b/src/spectator/tag_node_filter.cr @@ -10,7 +10,9 @@ module Spectator # Checks whether the node satisfies the filter. def includes?(node) : Bool - node.metadata.any? { |key, value| key.to_s == @tag && (!@value || value == @value) } + return false unless metadata = node.metadata + + metadata.any? { |key, value| key.to_s == @tag && (!@value || value == @value) } end end end diff --git a/src/spectator/test_context.cr b/src/spectator/test_context.cr index a68c5b9..e04fe56 100644 --- a/src/spectator/test_context.cr +++ b/src/spectator/test_context.cr @@ -34,7 +34,7 @@ class SpectatorTestContext < SpectatorContext # Initial metadata for tests. # This method should be overridden by example groups and examples. - private def self.metadata - ::Spectator::Metadata.new + private def self.metadata : ::Spectator::Metadata? + nil end end From 7ffa63718ba3e64b751351227a7b16de6e717922 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Thu, 8 Dec 2022 16:55:27 -0700 Subject: [PATCH 10/67] Use original type in redefinition comment --- src/spectator/mocks/stubbable.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spectator/mocks/stubbable.cr b/src/spectator/mocks/stubbable.cr index ffb52f3..4b03117 100644 --- a/src/spectator/mocks/stubbable.cr +++ b/src/spectator/mocks/stubbable.cr @@ -387,7 +387,7 @@ module Spectator method = definition[:method] scope = definition[:scope] receiver = definition[:receiver] %} - # Redefinition of {{type}}{{(receiver ? "." : "#").id}}{{method.name}} + # Redefinition of {{original_type}}{{(receiver ? "." : "#").id}}{{method.name}} {{(method.abstract? ? "abstract_stub abstract" : "default_stub").id}} {{method.visibility.id if method.visibility != :public}} def {{"#{receiver}.".id if receiver}}{{method.name}}( {% for arg, i in method.args %}{% if i == method.splat_index %}*{% end %}{{arg}}, {% end %} {% if method.double_splat %}**{{method.double_splat}}, {% end %} From 47a62ece789d8fe88bb05ef950078756c68dc299 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Thu, 8 Dec 2022 17:14:09 -0700 Subject: [PATCH 11/67] Add reduced test code for GitLab issue 80 https://gitlab.com/arctic-fox/spectator/-/issues/80 Note: This test only triggers a compiler bug when the file is compiled by itself. Compiling/running the entire spec suite *does not* cause the bug. --- spec/issues/gitlab_issue_80_spec.cr | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 spec/issues/gitlab_issue_80_spec.cr diff --git a/spec/issues/gitlab_issue_80_spec.cr b/spec/issues/gitlab_issue_80_spec.cr new file mode 100644 index 0000000..bdfe1ca --- /dev/null +++ b/spec/issues/gitlab_issue_80_spec.cr @@ -0,0 +1,29 @@ +require "../spec_helper" + +# https://gitlab.com/arctic-fox/spectator/-/issues/80# + +class Item +end + +class ItemUser + @item = Item.new + + def item + @item + end +end + +Spectator.describe "test1" do + it "without mock" do + item_user = ItemUser.new + item = item_user.item + item == item + end +end + +Spectator.describe "test2" do + mock Item do + end + it "without mock" do + end +end From bd44b5562e0a6e1a359294fd4013f2d50af0f460 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Fri, 9 Dec 2022 02:16:16 -0700 Subject: [PATCH 12/67] Possible fix for GitLab issue 80 Remove `is_a?` check on line 425. Replace with alternate logic that achieves the same thing. The `{{type}}` in `is_a?` was causing a compiler bug. I'm unsure of the root cause, but this works around it. --- src/spectator/mocks/stubbable.cr | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/spectator/mocks/stubbable.cr b/src/spectator/mocks/stubbable.cr index 4b03117..22c2c7a 100644 --- a/src/spectator/mocks/stubbable.cr +++ b/src/spectator/mocks/stubbable.cr @@ -422,12 +422,15 @@ module Spectator # If successful, which it will be in most cases, return it. # The caller will receive a properly typed value without unions or other side-effects. %cast = %value.as?({{type}}) - if %cast.is_a?({{type}}) + + {% if fail_cast == :nil %} %cast - else - {% if fail_cast == :nil %} - nil - {% elsif fail_cast == :raise %} + {% elsif fail_cast == :raise %} + # Check if nil was returned by the stub and if its okay to return it. + if %value.nil? && {{type}}.nilable? + # Value was nil and nil is allowed to be returned. + %cast.as({{type}}) + elsif %cast.nil? # The stubbed value was something else entirely and cannot be cast to the return type. # There's something weird going on (compiler bug?) that sometimes causes this class lookup to fail. %type = begin @@ -436,10 +439,13 @@ module Spectator "" end raise TypeCastError.new("#{_spectator_stubbed_name} received message #{ {{call}} } and is attempting to return a `#{%type}`, but returned type must be `#{ {{type}} }`.") - {% else %} - {% raise "fail_cast must be :nil, :raise, or :no_return, but got: #{fail_cast}" %} - {% end %} - end + else + # Types match and value can be returned as cast type. + %cast + end + {% else %} + {% raise "fail_cast must be :nil, :raise, or :no_return, but got: #{fail_cast}" %} + {% end %} {% end %} end end From 2985ef5919ae43d2f3f31ec2052e8a8da5380114 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Fri, 9 Dec 2022 02:22:21 -0700 Subject: [PATCH 13/67] Remove error handling around type resolution failure This might not be necessary anymore. --- src/spectator/mocks/stubbable.cr | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/spectator/mocks/stubbable.cr b/src/spectator/mocks/stubbable.cr index 22c2c7a..9b68c68 100644 --- a/src/spectator/mocks/stubbable.cr +++ b/src/spectator/mocks/stubbable.cr @@ -432,13 +432,7 @@ module Spectator %cast.as({{type}}) elsif %cast.nil? # The stubbed value was something else entirely and cannot be cast to the return type. - # There's something weird going on (compiler bug?) that sometimes causes this class lookup to fail. - %type = begin - %value.class.to_s - rescue - "" - end - raise TypeCastError.new("#{_spectator_stubbed_name} received message #{ {{call}} } and is attempting to return a `#{%type}`, but returned type must be `#{ {{type}} }`.") + raise TypeCastError.new("#{_spectator_stubbed_name} received message #{ {{call}} } and is attempting to return a `#{%value.class}`, but returned type must be `#{ {{type}} }`.") else # Types match and value can be returned as cast type. %cast From 293faccd5c781b1d113151ba6352146dce82e23a Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 13 Dec 2022 18:22:22 -0700 Subject: [PATCH 14/67] Support free variables in mocked types --- CHANGELOG.md | 3 ++ spec/issues/github_issue_48_spec.cr | 46 +++++++++++++++++++++++++++++ src/spectator/mocks/stubbable.cr | 11 ++++--- 3 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 spec/issues/github_issue_48_spec.cr diff --git a/CHANGELOG.md b/CHANGELOG.md index 704bc9e..a954fdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed +- Fix macro logic to support free variables on stubbed methods. + ### Changed - Simplify string representation of mock-related types. - Remove unnecessary redefinitions of methods when adding stub functionality to a type. diff --git a/spec/issues/github_issue_48_spec.cr b/spec/issues/github_issue_48_spec.cr new file mode 100644 index 0000000..6349a82 --- /dev/null +++ b/spec/issues/github_issue_48_spec.cr @@ -0,0 +1,46 @@ +require "../spec_helper" + +Spectator.describe "GitHub Issue #48" do + class Test + def return_this(thing : T) : T forall T + thing + end + + def map(thing : T, & : T -> U) : U forall T, U + yield thing + end + + def make_nilable(thing : T) : T? forall T + thing.as(T?) + end + end + + mock Test, make_nilable: nil + + let(fake) { mock(Test) } + + it "handles free variables" do + allow(fake).to receive(:return_this).and_return("different") + expect(fake.return_this("test")).to eq("different") + end + + it "raises on type cast error with free variables" do + allow(fake).to receive(:return_this).and_return(42) + expect { fake.return_this("test") }.to raise_error(TypeCastError, /String/) + end + + it "handles free variables with a block" do + allow(fake).to receive(:map).and_return("stub") + expect(fake.map(:mapped, &.to_s)).to eq("stub") + end + + it "raises on type cast error with a block and free variables" do + allow(fake).to receive(:map).and_return(42) + expect { fake.map(:mapped, &.to_s) }.to raise_error(TypeCastError, /String/) + end + + it "handles nilable free variables" do + fake = mock(Test) + expect(fake.make_nilable("foo")).to be_nil + end +end diff --git a/src/spectator/mocks/stubbable.cr b/src/spectator/mocks/stubbable.cr index 9b68c68..2e487d1 100644 --- a/src/spectator/mocks/stubbable.cr +++ b/src/spectator/mocks/stubbable.cr @@ -158,9 +158,11 @@ module Spectator # Cast the stub or return value to the expected type. # This is necessary to match the expected return type of the original method. _spectator_cast_stub_value(%stub, %call, typeof({{original}}), - {{ if method.return_type && method.return_type.resolve == NoReturn + {{ if method.return_type && method.return_type.resolve? == NoReturn :no_return - elsif method.return_type && method.return_type.resolve <= Nil || method.return_type.is_a?(Union) && method.return_type.types.map(&.resolve).includes?(Nil) + elsif method.return_type && + ((resolved = method.return_type.resolve?).is_a?(TypeNode) && resolved <= Nil) || + (method.return_type.is_a?(Union) && method.return_type.types.map(&.resolve?).includes?(Nil)) :nil else :raise @@ -262,9 +264,10 @@ module Spectator {% if method.return_type %} # Return type restriction takes priority since it can be a superset of the original implementation. _spectator_cast_stub_value(%stub, %call, {{method.return_type}}, - {{ if method.return_type.resolve == NoReturn + {{ if method.return_type.resolve? == NoReturn :no_return - elsif method.return_type.resolve <= Nil || method.return_type.is_a?(Union) && method.return_type.types.map(&.resolve).includes?(Nil) + elsif (method.return_type.resolve?.is_a?(TypeNode) && method.return_type.resolve <= Nil) || + (method.return_type.is_a?(Union) && method.return_type.types.map(&.resolve?).includes?(Nil)) :nil else :raise From 952e949307efee50edaa4c79e96a7ad83ce75af8 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 13 Dec 2022 22:48:21 -0700 Subject: [PATCH 15/67] Handle 'self' and some other variants in method return types --- CHANGELOG.md | 2 +- spec/issues/github_issue_48_spec.cr | 54 ++++++++++++++++++++++++++++- src/spectator/mocks/stubbable.cr | 45 +++++++++++++++++------- 3 files changed, 87 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a954fdd..d01c764 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed -- Fix macro logic to support free variables on stubbed methods. +- Fix macro logic to support free variables, 'self', and variants on stubbed methods. [#48](https://github.com/icy-arctic-fox/spectator/issues/48) ### Changed - Simplify string representation of mock-related types. diff --git a/spec/issues/github_issue_48_spec.cr b/spec/issues/github_issue_48_spec.cr index 6349a82..d5e68fb 100644 --- a/spec/issues/github_issue_48_spec.cr +++ b/spec/issues/github_issue_48_spec.cr @@ -13,6 +13,22 @@ Spectator.describe "GitHub Issue #48" do def make_nilable(thing : T) : T? forall T thing.as(T?) end + + def itself : self + self + end + + def itself? : self? + self.as(self?) + end + + def generic(thing : T) : Array(T) forall T + Array.new(100) { thing } + end + + def union : Int32 | String + 42.as(Int32 | String) + end end mock Test, make_nilable: nil @@ -40,7 +56,43 @@ Spectator.describe "GitHub Issue #48" do end it "handles nilable free variables" do - fake = mock(Test) expect(fake.make_nilable("foo")).to be_nil end + + it "handles 'self' return type" do + not_self = mock(Test) + allow(fake).to receive(:itself).and_return(not_self) + expect(fake.itself).to be(not_self) + end + + it "raises on type cast error with 'self' return type" do + allow(fake).to receive(:itself).and_return(42) + expect { fake.itself }.to raise_error(TypeCastError, /#{class_mock(Test)}/) + end + + it "handles nilable 'self' return type" do + not_self = mock(Test) + allow(fake).to receive(:itself?).and_return(not_self) + expect(fake.itself?).to be(not_self) + end + + it "handles generic return type" do + allow(fake).to receive(:generic).and_return([42]) + expect(fake.generic(42)).to eq([42]) + end + + it "raises on type cast error with generic return type" do + allow(fake).to receive(:generic).and_return("test") + expect { fake.generic(42) }.to raise_error(TypeCastError, /Array\(Int32\)/) + end + + it "handles union return types" do + allow(fake).to receive(:union).and_return("test") + expect(fake.union).to eq("test") + end + + it "raises on type cast error with union return type" do + allow(fake).to receive(:union).and_return(:test) + expect { fake.union }.to raise_error(TypeCastError, /Symbol/) + end end diff --git a/src/spectator/mocks/stubbable.cr b/src/spectator/mocks/stubbable.cr index 2e487d1..7c022cd 100644 --- a/src/spectator/mocks/stubbable.cr +++ b/src/spectator/mocks/stubbable.cr @@ -158,12 +158,24 @@ module Spectator # Cast the stub or return value to the expected type. # This is necessary to match the expected return type of the original method. _spectator_cast_stub_value(%stub, %call, typeof({{original}}), - {{ if method.return_type && method.return_type.resolve? == NoReturn - :no_return - elsif method.return_type && - ((resolved = method.return_type.resolve?).is_a?(TypeNode) && resolved <= Nil) || - (method.return_type.is_a?(Union) && method.return_type.types.map(&.resolve?).includes?(Nil)) - :nil + {{ if rt = method.return_type + if rt.is_a?(Path) && (resolved = rt.resolve?).is_a?(TypeNode) && resolved <= NoReturn + :no_return + else + # Process as an enumerable type to reduce code repetition. + rt = rt.is_a?(Union) ? rt.types : [rt] + # Check if any types are nilable. + nilable = rt.any? do |t| + # These are all macro types that have the `resolve?` method. + (t.is_a?(TypeNode) || t.is_a?(Path) || t.is_a?(Generic) || t.is_a?(MetaClass)) && + (resolved = t.resolve?).is_a?(TypeNode) && resolved <= Nil + end + if nilable + :nil + else + :raise + end + end else :raise end }}) @@ -261,16 +273,25 @@ module Spectator if %stub = _spectator_find_stub(%call) # Cast the stub or return value to the expected type. # This is necessary to match the expected return type of the original method. - {% if method.return_type %} + {% if rt = method.return_type %} # Return type restriction takes priority since it can be a superset of the original implementation. _spectator_cast_stub_value(%stub, %call, {{method.return_type}}, - {{ if method.return_type.resolve? == NoReturn + {{ if rt.is_a?(Path) && (resolved = rt.resolve?).is_a?(TypeNode) && resolved <= NoReturn :no_return - elsif (method.return_type.resolve?.is_a?(TypeNode) && method.return_type.resolve <= Nil) || - (method.return_type.is_a?(Union) && method.return_type.types.map(&.resolve?).includes?(Nil)) - :nil else - :raise + # Process as an enumerable type to reduce code repetition. + rt = rt.is_a?(Union) ? rt.types : [rt] + # Check if any types are nilable. + nilable = rt.any? do |t| + # These are all macro types that have the `resolve?` method. + (t.is_a?(TypeNode) || t.is_a?(Path) || t.is_a?(Generic) || t.is_a?(MetaClass)) && + (resolved = t.resolve?).is_a?(TypeNode) && resolved <= Nil + end + if nilable + :nil + else + :raise + end end }}) {% elsif !method.abstract? %} # The method isn't abstract, infer the type it returns without calling it. From 7e2ec4ee379364b5621cd02558a62602b0eef505 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 13 Dec 2022 22:59:42 -0700 Subject: [PATCH 16/67] Fix 0.11.4 in changelog --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d01c764..e700a80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Remove unnecessary redefinitions of methods when adding stub functionality to a type. - Allow metadata to be stored as nil to reduce overhead when tracking nodes without tags. -## [0.11.4] +## [0.11.4] - 2022-11-27 ### Added - Add support for using named (keyword) arguments in place of positional arguments in stubs. [#47](https://github.com/icy-arctic-fox/spectator/issues/47) - Add `before`, `after`, and `around` as aliases for `before_each`, `after_each`, and `around_each` respectively. @@ -426,7 +426,7 @@ First version ready for public use. [Unreleased]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.4...master -[0.11.3]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.3...v0.11.4 +[0.11.4]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.3...v0.11.4 [0.11.3]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.2...v0.11.3 [0.11.2]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.1...v0.11.2 [0.11.1]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.0...v0.11.1 From b52593dbded131f6b019b3bec3d8ca20456ccf84 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 17 Dec 2022 16:39:47 -0700 Subject: [PATCH 17/67] Cleanup --- spec/issues/gitlab_issue_80_spec.cr | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/issues/gitlab_issue_80_spec.cr b/spec/issues/gitlab_issue_80_spec.cr index bdfe1ca..9090130 100644 --- a/spec/issues/gitlab_issue_80_spec.cr +++ b/spec/issues/gitlab_issue_80_spec.cr @@ -1,6 +1,6 @@ require "../spec_helper" -# https://gitlab.com/arctic-fox/spectator/-/issues/80# +# https://gitlab.com/arctic-fox/spectator/-/issues/80 class Item end @@ -24,6 +24,7 @@ end Spectator.describe "test2" do mock Item do end + it "without mock" do end end From 65a4b8e756f36d57cd725c94aa60ca1673a83ab2 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 17 Dec 2022 16:41:22 -0700 Subject: [PATCH 18/67] Populate previous_def/super with captured block args The previous_def and super keywords do not propagate blocks. See: https://github.com/crystal-lang/crystal/issues/10399 This works around the issue by populating arguments if the method uses a block. --- spec/issues/github_issue_48_spec.cr | 27 ++++++++++++ src/spectator/mocks/stubbable.cr | 64 +++++++++++++++++++++++++++-- 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/spec/issues/github_issue_48_spec.cr b/spec/issues/github_issue_48_spec.cr index d5e68fb..21e4664 100644 --- a/spec/issues/github_issue_48_spec.cr +++ b/spec/issues/github_issue_48_spec.cr @@ -29,6 +29,15 @@ Spectator.describe "GitHub Issue #48" do def union : Int32 | String 42.as(Int32 | String) end + + def capture(&block : -> T) forall T + block + end + + def capture(thing : T, &block : T -> T) forall T + block.call(thing) + block + end end mock Test, make_nilable: nil @@ -95,4 +104,22 @@ Spectator.describe "GitHub Issue #48" do allow(fake).to receive(:union).and_return(:test) expect { fake.union }.to raise_error(TypeCastError, /Symbol/) end + + it "handles captured blocks" do + proc = ->{} + allow(fake).to receive(:capture).and_return(proc) + expect(fake.capture { nil }).to be(proc) + end + + it "raises on type cast error with captured blocks" do + proc = ->{ 42 } + allow(fake).to receive(:capture).and_return(proc) + expect { fake.capture { "other" } }.to raise_error(TypeCastError, /Proc\(String\)/) + end + + it "handles captured blocks with arguments" do + proc = ->(x : Int32) { x * 2 } + allow(fake).to receive(:capture).and_return(proc) + expect(fake.capture(5) { 5 }).to be(proc) + end end diff --git a/src/spectator/mocks/stubbable.cr b/src/spectator/mocks/stubbable.cr index 7c022cd..6ded8ee 100644 --- a/src/spectator/mocks/stubbable.cr +++ b/src/spectator/mocks/stubbable.cr @@ -126,7 +126,31 @@ module Spectator {{method.body}} end - {% original = "previous_def#{" { |*_spectator_yargs| yield *_spectator_yargs }".id if method.accepts_block?}".id %} + {% original = "previous_def" + # Workaround for Crystal not propagating block with previous_def/super. + if method.accepts_block? + original += "(" + method.args.each_with_index do |arg, i| + original += '*' if method.splat_index == i + original += arg.name.stringify + original += ", " + end + if method.double_splat + original += method.double_splat.stringify + original += ", " + end + # If the block is captured (i.e. `&block` syntax), it must be passed along as an argument. + # Otherwise, use `yield` to forward the block. + captured_block = if method.block_arg && method.block_arg.name && method.block_arg.name.size > 0 + method.block_arg.name + else + nil + end + original += "&#{captured_block}" if captured_block + original += ")" + original += " { |*_spectator_yargs| yield *_spectator_yargs }" unless captured_block + end + original = original.id %} {% # Reconstruct the method signature. # I wish there was a better way of doing this, but there isn't (at least not that I'm aware of). @@ -241,7 +265,32 @@ module Spectator {{method.body}} end - {% original = "previous_def#{" { |*_spectator_yargs| yield *_spectator_yargs }".id if method.accepts_block?}".id %} + {% original = "previous_def" + # Workaround for Crystal not propagating block with previous_def/super. + if method.accepts_block? + original += "(" + method.args.each_with_index do |arg, i| + original += '*' if method.splat_index == i + original += arg.name.stringify + original += ", " + end + if method.double_splat + original += method.double_splat.stringify + original += ", " + end + # If the block is captured (i.e. `&block` syntax), it must be passed along as an argument. + # Otherwise, use `yield` to forward the block. + captured_block = if method.block_arg && method.block_arg.name && method.block_arg.name.size > 0 + method.block_arg.name + else + nil + end + original += "&#{captured_block}" if captured_block + original += ")" + original += " { |*_spectator_yargs| yield *_spectator_yargs }" unless captured_block + end + original = original.id %} + {% end %} {% # Reconstruct the method signature. @@ -418,7 +467,16 @@ module Spectator {% if method.block_arg %}&{{method.block_arg}}{% elsif method.accepts_block? %}&{% end %} ){% if method.return_type %} : {{method.return_type}}{% end %}{% if !method.free_vars.empty? %} forall {{method.free_vars.splat}}{% end %} {% unless method.abstract? %} - {{scope}}{% if method.accepts_block? %} { |*%yargs| yield *%yargs }{% end %} + {{scope}}{% if method.accepts_block? %}( + {% for arg, i in method.args %}{% if i == method.splat_index %}*{% end %}{{arg.name}}, {% end %} + {% if method.double_splat %}**{{method.double_splat}}, {% end %} + {% captured_block = if method.block_arg && method.block_arg.name && method.block_arg.name.size > 0 + method.block_arg.name + else + nil + end %} + {% if captured_block %}&{{captured_block}}{% end %} + ){% if !captured_block %} { |*%yargs| yield *%yargs }{% end %}{% end %} end {% end %} {% end %} From 9f54a9e542b46e75b74ac3989ae1b0f8b09c8043 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 17 Dec 2022 19:16:38 -0700 Subject: [PATCH 19/67] Additional handling for passing blocks --- CHANGELOG.md | 1 + src/spectator/mocks/stubbable.cr | 62 ++++++++++++++++++++++---------- 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e700a80..645a115 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed - Fix macro logic to support free variables, 'self', and variants on stubbed methods. [#48](https://github.com/icy-arctic-fox/spectator/issues/48) +- Fix method stubs used on methods that capture blocks. ### Changed - Simplify string representation of mock-related types. diff --git a/src/spectator/mocks/stubbable.cr b/src/spectator/mocks/stubbable.cr index 6ded8ee..9c32c47 100644 --- a/src/spectator/mocks/stubbable.cr +++ b/src/spectator/mocks/stubbable.cr @@ -130,14 +130,25 @@ module Spectator # Workaround for Crystal not propagating block with previous_def/super. if method.accepts_block? original += "(" - method.args.each_with_index do |arg, i| - original += '*' if method.splat_index == i - original += arg.name.stringify - original += ", " - end - if method.double_splat - original += method.double_splat.stringify - original += ", " + if method.splat_index + method.args.each_with_index do |arg, i| + if i == method.splat_index + original += '*' + if arg.internal_name && arg.internal_name.size > 0 + original += "#{arg.internal_name}, " + end + original += "**#{method.double_splat}, " if method.double_splat + elsif i > method.splat_index + original += "#{arg.name}: #{arg.internal_name}" + else + original += "#{arg.internal_name}, " + end + end + else + method.args.each do |arg| + original += "#{arg.internal_name}, " + end + original += "**#{method.double_splat}, " if method.double_splat end # If the block is captured (i.e. `&block` syntax), it must be passed along as an argument. # Otherwise, use `yield` to forward the block. @@ -269,14 +280,25 @@ module Spectator # Workaround for Crystal not propagating block with previous_def/super. if method.accepts_block? original += "(" - method.args.each_with_index do |arg, i| - original += '*' if method.splat_index == i - original += arg.name.stringify - original += ", " - end - if method.double_splat - original += method.double_splat.stringify - original += ", " + if method.splat_index + method.args.each_with_index do |arg, i| + if i == method.splat_index + original += '*' + if arg.internal_name && arg.internal_name.size > 0 + original += "#{arg.internal_name}, " + end + original += "**#{method.double_splat}, " if method.double_splat + elsif i > method.splat_index + original += "#{arg.name}: #{arg.internal_name}" + else + original += "#{arg.internal_name}, " + end + end + else + method.args.each do |arg| + original += "#{arg.internal_name}, " + end + original += "**#{method.double_splat}, " if method.double_splat end # If the block is captured (i.e. `&block` syntax), it must be passed along as an argument. # Otherwise, use `yield` to forward the block. @@ -467,9 +489,11 @@ module Spectator {% if method.block_arg %}&{{method.block_arg}}{% elsif method.accepts_block? %}&{% end %} ){% if method.return_type %} : {{method.return_type}}{% end %}{% if !method.free_vars.empty? %} forall {{method.free_vars.splat}}{% end %} {% unless method.abstract? %} - {{scope}}{% if method.accepts_block? %}( - {% for arg, i in method.args %}{% if i == method.splat_index %}*{% end %}{{arg.name}}, {% end %} - {% if method.double_splat %}**{{method.double_splat}}, {% end %} + {{scope}}{% if method.accepts_block? %}({% for arg, i in method.args %} + {% if i == method.splat_index && arg.internal_name && arg.internal_name.size > 0 %}*{{arg.internal_name}}, {% if method.double_splat %}**{{method.double_splat}}, {% end %}{% end %} + {% if method.splat_index && i > method.splat_index %}{{arg.name}}: {{arg.internal_name}}, {% end %} + {% if !method.splat_index || i < method.splat_index %}{{arg.internal_name}}, {% end %}{% end %} + {% if !method.splat_index && method.double_splat %}**{{method.double_splat}}, {% end %} {% captured_block = if method.block_arg && method.block_arg.name && method.block_arg.name.size > 0 method.block_arg.name else From 149c0e6e4b7e6ab4de0e16a84a0f45183131c634 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 17 Dec 2022 19:19:33 -0700 Subject: [PATCH 20/67] Don't use case-matching for proc arguments A proc on the left side of === calls itself passing in the right side. This causes typing issues and is easier to avoid for now. Procs arguments are compared with standard equality (==) instead of case-equality (===). --- CHANGELOG.md | 1 + src/spectator/mocks/abstract_arguments.cr | 32 ++++++++++++++++++++++- src/spectator/mocks/arguments.cr | 14 +++++++--- src/spectator/mocks/formal_arguments.cr | 4 +-- 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 645a115..248f805 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Simplify string representation of mock-related types. - Remove unnecessary redefinitions of methods when adding stub functionality to a type. - Allow metadata to be stored as nil to reduce overhead when tracking nodes without tags. +- Use normal equality (==) instead of case-equality (===) with proc arguments in stubs. ## [0.11.4] - 2022-11-27 ### Added diff --git a/src/spectator/mocks/abstract_arguments.cr b/src/spectator/mocks/abstract_arguments.cr index b03f161..b06934c 100644 --- a/src/spectator/mocks/abstract_arguments.cr +++ b/src/spectator/mocks/abstract_arguments.cr @@ -6,11 +6,41 @@ module Spectator to_s(io) end + # Utility method for comparing two tuples considering special types. + private def compare_tuples(a : Tuple, b : Tuple) + return false if a.size != b.size + + a.zip(b) do |a_value, b_value| + if a_value.is_a?(Proc) + # Using procs as argument matchers isn't supported currently. + # Compare directly instead. + return false unless a_value == b_value + else + return false unless a_value === b_value + end + end + true + end + + # Utility method for comparing two tuples considering special types. + # Supports nilable tuples (ideal for splats). + private def compare_tuples(a : Tuple?, b : Tuple?) + return false if a.nil? ^ b.nil? + + compare_tuples(a.not_nil!, b.not_nil!) + end + # Utility method for comparing two named tuples ignoring order. private def compare_named_tuples(a : NamedTuple, b : NamedTuple) a.each do |k, v1| v2 = b.fetch(k) { return false } - return false unless v1 === v2 + if v1.is_a?(Proc) + # Using procs as argument matchers isn't supported currently. + # Compare directly instead. + return false unless v1 == v2 + else + return false unless v1 === v2 + end end true end diff --git a/src/spectator/mocks/arguments.cr b/src/spectator/mocks/arguments.cr index fcb74f4..2638e77 100644 --- a/src/spectator/mocks/arguments.cr +++ b/src/spectator/mocks/arguments.cr @@ -81,7 +81,7 @@ module Spectator # Checks if another set of arguments matches this set of arguments. def ===(other : Arguments) - positional === other.positional && compare_named_tuples(kwargs, other.kwargs) + compare_tuples(positional, other.positional) && compare_named_tuples(kwargs, other.kwargs) end # :ditto: @@ -95,13 +95,21 @@ module Spectator v1 = positional[i] i += 1 - return false unless v1 === v2 + if v1.is_a?(Proc) + return false unless v1 == v2 + else + return false unless v1 === v2 + end end other.splat.try &.each do |v2| v1 = positional.fetch(i) { return false } i += 1 - return false unless v1 === v2 + if v1.is_a?(Proc) + return false unless v1 == v2 + else + return false unless v1 === v2 + end end i == positional.size diff --git a/src/spectator/mocks/formal_arguments.cr b/src/spectator/mocks/formal_arguments.cr index e440214..1c0ca69 100644 --- a/src/spectator/mocks/formal_arguments.cr +++ b/src/spectator/mocks/formal_arguments.cr @@ -122,12 +122,12 @@ module Spectator # Checks if another set of arguments matches this set of arguments. def ===(other : Arguments) - positional === other.positional && compare_named_tuples(kwargs, other.kwargs) + compare_tuples(positional, other.positional) && compare_named_tuples(kwargs, other.kwargs) end # :ditto: def ===(other : FormalArguments) - compare_named_tuples(args, other.args) && splat === other.splat && compare_named_tuples(kwargs, other.kwargs) + compare_named_tuples(args, other.args) && compare_tuples(splat, other.splat) && compare_named_tuples(kwargs, other.kwargs) end end end From c3e7edc7003546d607ea9784ff7009d70095b090 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 17 Dec 2022 20:37:27 -0700 Subject: [PATCH 21/67] Use absolute names of types in mocked type methods Prevent possibly type name collisions. This could happen if, for instance, Array or String was redefined in the scope of the mocked type. --- src/spectator/mocks/mock.cr | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/spectator/mocks/mock.cr b/src/spectator/mocks/mock.cr index ba72b37..a5d6128 100644 --- a/src/spectator/mocks/mock.cr +++ b/src/spectator/mocks/mock.cr @@ -50,22 +50,22 @@ module Spectator end {% end %} - def _spectator_remove_stub(stub : ::Spectator::Stub) : Nil + def _spectator_remove_stub(stub : ::Spectator::Stub) : ::Nil @_spectator_stubs.try &.delete(stub) end - def _spectator_clear_stubs : Nil + def _spectator_clear_stubs : ::Nil @_spectator_stubs = nil end - private class_getter _spectator_stubs : Array(::Spectator::Stub) = [] of ::Spectator::Stub + private class_getter _spectator_stubs : ::Array(::Spectator::Stub) = [] of ::Spectator::Stub - class_getter _spectator_calls : Array(::Spectator::MethodCall) = [] of ::Spectator::MethodCall + class_getter _spectator_calls : ::Array(::Spectator::MethodCall) = [] of ::Spectator::MethodCall getter _spectator_calls = [] of ::Spectator::MethodCall # Returns the mock's name formatted for user output. - private def _spectator_stubbed_name : String + private def _spectator_stubbed_name : ::String \{% if anno = @type.annotation(::Spectator::StubbedName) %} "#" \{% else %} @@ -73,7 +73,7 @@ module Spectator \{% end %} end - private def self._spectator_stubbed_name : String + private def self._spectator_stubbed_name : ::String \{% if anno = @type.annotation(::Spectator::StubbedName) %} "#" \{% else %} @@ -132,9 +132,9 @@ module Spectator {% raise "Unsupported base type #{base} for injecting mock" %} {% end %} - private class_getter _spectator_stubs : Array(::Spectator::Stub) = [] of ::Spectator::Stub + private class_getter _spectator_stubs : ::Array(::Spectator::Stub) = [] of ::Spectator::Stub - class_getter _spectator_calls : Array(::Spectator::MethodCall) = [] of ::Spectator::MethodCall + class_getter _spectator_calls : ::Array(::Spectator::MethodCall) = [] of ::Spectator::MethodCall private def _spectator_stubs entry = @@_spectator_mock_registry.fetch(self) do @@ -143,11 +143,11 @@ module Spectator entry.stubs end - def _spectator_remove_stub(stub : ::Spectator::Stub) : Nil + def _spectator_remove_stub(stub : ::Spectator::Stub) : ::Nil @@_spectator_mock_registry[self]?.try &.stubs.delete(stub) end - def _spectator_clear_stubs : Nil + def _spectator_clear_stubs : ::Nil @@_spectator_mock_registry.delete(self) end @@ -169,7 +169,7 @@ module Spectator end # Returns the mock's name formatted for user output. - private def _spectator_stubbed_name : String + private def _spectator_stubbed_name : ::String \{% if anno = @type.annotation(::Spectator::StubbedName) %} "#" \{% else %} @@ -178,7 +178,7 @@ module Spectator end # Returns the mock's name formatted for user output. - private def self._spectator_stubbed_name : String + private def self._spectator_stubbed_name : ::String \{% if anno = @type.annotation(::Spectator::StubbedName) %} "#" \{% else %} From 4b68b8e3de1285add515e9521a979cc0f94dd818 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 17 Dec 2022 20:56:16 -0700 Subject: [PATCH 22/67] Fix resolution issue when mocked types use custom types GitLab issue 51 is affected. https://gitlab.com/arctic-fox/spectator/-/issues/51 Private types cannot be referenced with mocks. --- CHANGELOG.md | 1 + spec/issues/gitlab_issue_51_spec.cr | 42 +++++++------- spec/spectator/mocks/mock_spec.cr | 90 +++++++++++++++++++++++++++++ src/spectator/dsl/mocks.cr | 35 ++++++----- 4 files changed, 133 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 248f805..46800cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix macro logic to support free variables, 'self', and variants on stubbed methods. [#48](https://github.com/icy-arctic-fox/spectator/issues/48) - Fix method stubs used on methods that capture blocks. +- Fix type name resolution for when using custom types in a mocked typed. ### Changed - Simplify string representation of mock-related types. diff --git a/spec/issues/gitlab_issue_51_spec.cr b/spec/issues/gitlab_issue_51_spec.cr index c809607..996af80 100644 --- a/spec/issues/gitlab_issue_51_spec.cr +++ b/spec/issues/gitlab_issue_51_spec.cr @@ -1,31 +1,33 @@ require "../spec_helper" -private class Foo - def call(str : String) : String? - "" +module GitLabIssue51 + class Foo + def call(str : String) : String? + "" + end + + def alt1_call(str : String) : String? + nil + end + + def alt2_call(str : String) : String? + [str, nil].sample + end end - def alt1_call(str : String) : String? - nil - end - - def alt2_call(str : String) : String? - [str, nil].sample + class Bar + def call(a_foo) : Nil # Must add nil restriction here, otherwise a segfault occurs from returning the result of #alt2_call. + a_foo.call("") + a_foo.alt1_call("") + a_foo.alt2_call("") + end end end -private class Bar - def call(a_foo) : Nil # Must add nil restriction here, otherwise a segfault occurs from returning the result of #alt2_call. - a_foo.call("") - a_foo.alt1_call("") - a_foo.alt2_call("") - end -end +Spectator.describe GitLabIssue51::Bar do + mock GitLabIssue51::Foo, call: "", alt1_call: "", alt2_call: "" -Spectator.describe Bar do - mock Foo, call: "", alt1_call: "", alt2_call: "" - - let(:foo) { mock(Foo) } + let(:foo) { mock(GitLabIssue51::Foo) } subject(:call) { described_class.new.call(foo) } describe "#call" do diff --git a/spec/spectator/mocks/mock_spec.cr b/spec/spectator/mocks/mock_spec.cr index 40b4bf0..8835183 100644 --- a/spec/spectator/mocks/mock_spec.cr +++ b/spec/spectator/mocks/mock_spec.cr @@ -29,8 +29,18 @@ Spectator.describe Spectator::Mock do @_spectator_invocations << :method3 "original" end + + def method4 : Thing + self + end + + def method5 : OtherThing + OtherThing.new + end end + class OtherThing; end + Spectator::Mock.define_subtype(:class, Thing, MockThing, :mock_name, method1: 123) do stub def method2 :stubbed @@ -104,6 +114,20 @@ Spectator.describe Spectator::Mock do mock.method3 expect(mock._spectator_invocations).to contain_exactly(:method3) end + + it "can reference its own type" do + new_mock = MockThing.new + stub = Spectator::ValueStub.new(:method4, new_mock) + mock._spectator_define_stub(stub) + expect(mock.method4).to be(new_mock) + end + + it "can reference other types in the original namespace" do + other = OtherThing.new + stub = Spectator::ValueStub.new(:method5, other) + mock._spectator_define_stub(stub) + expect(mock.method5).to be(other) + end end context "with an abstract class" do @@ -120,8 +144,14 @@ Spectator.describe Spectator::Mock do end abstract def method4 + + abstract def method4 : Thing + + abstract def method5 : OtherThing end + class OtherThing; end + Spectator::Mock.define_subtype(:class, Thing, MockThing, :mock_name, method2: :stubbed) do stub def method1 : Int32 # NOTE: Return type is required since one wasn't provided in the parent. 123 @@ -199,6 +229,20 @@ Spectator.describe Spectator::Mock do mock.method3 expect(mock._spectator_invocations).to contain_exactly(:method3) end + + it "can reference its own type" do + new_mock = MockThing.new + stub = Spectator::ValueStub.new(:method4, new_mock) + mock._spectator_define_stub(stub) + expect(mock.method4).to be(new_mock) + end + + it "can reference other types in the original namespace" do + other = OtherThing.new + stub = Spectator::ValueStub.new(:method5, other) + mock._spectator_define_stub(stub) + expect(mock.method5).to be(other) + end end context "with an abstract struct" do @@ -215,8 +259,14 @@ Spectator.describe Spectator::Mock do end abstract def method4 + + abstract def method4 : Thing + + abstract def method5 : OtherThing end + class OtherThing; end + Spectator::Mock.define_subtype(:struct, Thing, MockThing, :mock_name, method2: :stubbed) do stub def method1 : Int32 # NOTE: Return type is required since one wasn't provided in the parent. 123 @@ -286,6 +336,22 @@ Spectator.describe Spectator::Mock do mock.method3 expect(mock._spectator_invocations).to contain_exactly(:method3) end + + it "can reference its own type" do + mock = self.mock # FIXME: Workaround for passing by value messing with stubs. + new_mock = MockThing.new + stub = Spectator::ValueStub.new(:method4, new_mock) + mock._spectator_define_stub(stub) + expect(mock.method4).to be_a(Thing) + end + + it "can reference other types in the original namespace" do + mock = self.mock # FIXME: Workaround for passing by value messing with stubs. + other = OtherThing.new + stub = Spectator::ValueStub.new(:method5, other) + mock._spectator_define_stub(stub) + expect(mock.method5).to be(other) + end end context "class method stubs" do @@ -301,8 +367,18 @@ Spectator.describe Spectator::Mock do def self.baz(arg) yield end + + def self.thing : Thing + new + end + + def self.other : OtherThing + OtherThing.new + end end + class OtherThing; end + Spectator::Mock.define_subtype(:class, Thing, MockThing) do stub def self.foo :stub @@ -367,6 +443,20 @@ Spectator.describe Spectator::Mock do expect(restricted(mock)).to eq(:stub) end + it "can reference its own type" do + new_mock = MockThing.new + stub = Spectator::ValueStub.new(:thing, new_mock) + mock._spectator_define_stub(stub) + expect(mock.thing).to be(new_mock) + end + + it "can reference other types in the original namespace" do + other = OtherThing.new + stub = Spectator::ValueStub.new(:other, other) + mock._spectator_define_stub(stub) + expect(mock.other).to be(other) + end + describe "._spectator_clear_stubs" do before { mock._spectator_define_stub(foo_stub) } diff --git a/src/spectator/dsl/mocks.cr b/src/spectator/dsl/mocks.cr index 94a41f9..ab05636 100644 --- a/src/spectator/dsl/mocks.cr +++ b/src/spectator/dsl/mocks.cr @@ -218,24 +218,29 @@ module Spectator::DSL # end # ``` private macro def_mock(type, name = nil, **value_methods, &block) - {% # Construct a unique type name for the mock by using the number of defined types. - index = ::Spectator::DSL::Mocks::TYPES.size - mock_type_name = "Mock#{index}".id + {% resolved = type.resolve + # Construct a unique type name for the mock by using the number of defined types. + index = ::Spectator::DSL::Mocks::TYPES.size + # The type is nested under the original so that any type names from the original can be resolved. + mock_type_name = "Mock#{index}".id - # Store information about how the mock is defined and its context. - # This is important for constructing an instance of the mock later. - ::Spectator::DSL::Mocks::TYPES << {type.id.symbolize, @type.name(generic_args: false).symbolize, mock_type_name.symbolize} + # Store information about how the mock is defined and its context. + # This is important for constructing an instance of the mock later. + ::Spectator::DSL::Mocks::TYPES << {type.id.symbolize, @type.name(generic_args: false).symbolize, "::#{resolved.name}::#{mock_type_name}".id.symbolize} - resolved = type.resolve - base = if resolved.class? - :class - elsif resolved.struct? - :struct - else - :module - end %} + base = if resolved.class? + :class + elsif resolved.struct? + :struct + else + :module + end %} - ::Spectator::Mock.define_subtype({{base}}, {{type.id}}, {{mock_type_name}}, {{name}}, {{**value_methods}}) {{block}} + {% begin %} + {{base.id}} ::{{resolved.name}} + ::Spectator::Mock.define_subtype({{base}}, {{type.id}}, {{mock_type_name}}, {{name}}, {{**value_methods}}) {{block}} + end + {% end %} end # Instantiates a mock. From f55c60e01fcf1559cc4420353e3f6b4a6992714b Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 17 Dec 2022 21:01:22 -0700 Subject: [PATCH 23/67] Fix README spec Mocked types cannot be private. Moved to a module to prevent polluting the global namespace. --- spec/docs/readme_spec.cr | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/spec/docs/readme_spec.cr b/spec/docs/readme_spec.cr index 6024906..2ed7fd5 100644 --- a/spec/docs/readme_spec.cr +++ b/spec/docs/readme_spec.cr @@ -1,26 +1,28 @@ require "../spec_helper" -private abstract class Interface - abstract def invoke(thing) : String -end +module Readme + abstract class Interface + abstract def invoke(thing) : String + end -# Type being tested. -private class Driver - def do_something(interface : Interface, thing) - interface.invoke(thing) + # Type being tested. + class Driver + def do_something(interface : Interface, thing) + interface.invoke(thing) + end end end -Spectator.describe Driver do +Spectator.describe Readme::Driver do # Define a mock for Interface. - mock Interface + mock Readme::Interface # Define a double that the interface will use. double(:my_double, foo: 42) it "does a thing" do # Create an instance of the mock interface. - interface = mock(Interface) + interface = mock(Readme::Interface) # Indicate that `#invoke` should return "test" when called. allow(interface).to receive(:invoke).and_return("test") From e6584c9f042fd111d870a03a2924d87b8cbcfcb7 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sun, 18 Dec 2022 11:35:43 -0700 Subject: [PATCH 24/67] Prevent comparing range arguments with non-compatible types in stubs Addresses https://github.com/icy-arctic-fox/spectator/issues/48 --- CHANGELOG.md | 1 + spec/issues/github_issue_48_spec.cr | 10 ++++++ src/spectator/mocks/abstract_arguments.cr | 38 ++++++++++++++--------- src/spectator/mocks/arguments.cr | 12 ++----- 4 files changed, 37 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46800cf..73ffadb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix macro logic to support free variables, 'self', and variants on stubbed methods. [#48](https://github.com/icy-arctic-fox/spectator/issues/48) - Fix method stubs used on methods that capture blocks. - Fix type name resolution for when using custom types in a mocked typed. +- Prevent comparing range arguments with non-compatible types in stubs. [#48](https://github.com/icy-arctic-fox/spectator/issues/48) ### Changed - Simplify string representation of mock-related types. diff --git a/spec/issues/github_issue_48_spec.cr b/spec/issues/github_issue_48_spec.cr index 21e4664..b958c1b 100644 --- a/spec/issues/github_issue_48_spec.cr +++ b/spec/issues/github_issue_48_spec.cr @@ -38,6 +38,10 @@ Spectator.describe "GitHub Issue #48" do block.call(thing) block end + + def range(r : Range) + r + end end mock Test, make_nilable: nil @@ -122,4 +126,10 @@ Spectator.describe "GitHub Issue #48" do allow(fake).to receive(:capture).and_return(proc) expect(fake.capture(5) { 5 }).to be(proc) end + + it "handles range comparisons against non-comparable types" do + range = 1..10 + allow(fake).to receive(:range).and_return(range) + expect(fake.range(1..3)).to eq(range) + end end diff --git a/src/spectator/mocks/abstract_arguments.cr b/src/spectator/mocks/abstract_arguments.cr index b06934c..442c1d2 100644 --- a/src/spectator/mocks/abstract_arguments.cr +++ b/src/spectator/mocks/abstract_arguments.cr @@ -11,13 +11,7 @@ module Spectator return false if a.size != b.size a.zip(b) do |a_value, b_value| - if a_value.is_a?(Proc) - # Using procs as argument matchers isn't supported currently. - # Compare directly instead. - return false unless a_value == b_value - else - return false unless a_value === b_value - end + return false unless compare_values(a_value, b_value) end true end @@ -34,15 +28,31 @@ module Spectator private def compare_named_tuples(a : NamedTuple, b : NamedTuple) a.each do |k, v1| v2 = b.fetch(k) { return false } - if v1.is_a?(Proc) - # Using procs as argument matchers isn't supported currently. - # Compare directly instead. - return false unless v1 == v2 - else - return false unless v1 === v2 - end + return false unless compare_values(v1, v2) end true end + + # Utility method for comparing two arguments considering special types. + # Some types used for case-equality don't work well with unexpected right-hand types. + # This can happen when the right side is a massive union of types. + private def compare_values(a, b) + case a + when Proc + # Using procs as argument matchers isn't supported currently. + # Compare directly instead. + a == b + when Range + # Ranges can only be matched against if their right side is comparable. + # Ensure the right side is comparable, otherwise compare directly. + if b.is_a?(Comparable(typeof(b))) + a === b + else + a == b + end + else + a === b + end + end end end diff --git a/src/spectator/mocks/arguments.cr b/src/spectator/mocks/arguments.cr index 2638e77..f15a0ba 100644 --- a/src/spectator/mocks/arguments.cr +++ b/src/spectator/mocks/arguments.cr @@ -95,21 +95,13 @@ module Spectator v1 = positional[i] i += 1 - if v1.is_a?(Proc) - return false unless v1 == v2 - else - return false unless v1 === v2 - end + return false unless compare_values(v1, v2) end other.splat.try &.each do |v2| v1 = positional.fetch(i) { return false } i += 1 - if v1.is_a?(Proc) - return false unless v1 == v2 - else - return false unless v1 === v2 - end + return false unless compare_values(v1, v2) end i == positional.size From 6255cc85c44e4a5e91afaa469ae154026b07937e Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sun, 18 Dec 2022 15:17:48 -0700 Subject: [PATCH 25/67] Handle original call reaching to another type Primary use case for this is mock modules. Allows default stubs to access more than previous_def and super. --- src/spectator/mocks/stubbable.cr | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/spectator/mocks/stubbable.cr b/src/spectator/mocks/stubbable.cr index 9c32c47..48f6ac5 100644 --- a/src/spectator/mocks/stubbable.cr +++ b/src/spectator/mocks/stubbable.cr @@ -434,7 +434,13 @@ module Spectator private macro stub_type(type_name = @type) {% type = type_name.resolve definitions = [] of Nil - scope = (type == @type ? :previous_def : :super).id + scope = if type == @type + :previous_def + elsif type.module? + type.name + else + :super + end.id # Add entries for methods in the target type and its class type. [[:self.id, type.class], [nil, type]].each do |(receiver, t)| @@ -481,15 +487,25 @@ module Spectator {% original_type = definition[:type] method = definition[:method] scope = definition[:scope] - receiver = definition[:receiver] %} - # Redefinition of {{original_type}}{{(receiver ? "." : "#").id}}{{method.name}} + receiver = definition[:receiver] + rewrite_args = method.accepts_block? + # Handle calling methods on other objects (primarily for mock modules). + if scope != :super.id && scope != :previous_def.id + if receiver == :self.id + scope = "#{scope}.#{method.name}".id + rewrite_args = true + else + scope = :super.id + end + end %} + # Redefinition of {{original_type}}{{"#".id}}{{method.name}} {{(method.abstract? ? "abstract_stub abstract" : "default_stub").id}} {{method.visibility.id if method.visibility != :public}} def {{"#{receiver}.".id if receiver}}{{method.name}}( {% for arg, i in method.args %}{% if i == method.splat_index %}*{% end %}{{arg}}, {% end %} {% if method.double_splat %}**{{method.double_splat}}, {% end %} {% if method.block_arg %}&{{method.block_arg}}{% elsif method.accepts_block? %}&{% end %} ){% if method.return_type %} : {{method.return_type}}{% end %}{% if !method.free_vars.empty? %} forall {{method.free_vars.splat}}{% end %} {% unless method.abstract? %} - {{scope}}{% if method.accepts_block? %}({% for arg, i in method.args %} + {{scope}}{% if rewrite_args %}({% for arg, i in method.args %} {% if i == method.splat_index && arg.internal_name && arg.internal_name.size > 0 %}*{{arg.internal_name}}, {% if method.double_splat %}**{{method.double_splat}}, {% end %}{% end %} {% if method.splat_index && i > method.splat_index %}{{arg.name}}: {{arg.internal_name}}, {% end %} {% if !method.splat_index || i < method.splat_index %}{{arg.internal_name}}, {% end %}{% end %} @@ -500,7 +516,7 @@ module Spectator nil end %} {% if captured_block %}&{{captured_block}}{% end %} - ){% if !captured_block %} { |*%yargs| yield *%yargs }{% end %}{% end %} + ){% if !captured_block && method.accepts_block? %} { |*%yargs| yield *%yargs }{% end %}{% end %} end {% end %} {% end %} From d37858305403a3339852867996e26cd4dd2293b0 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sun, 18 Dec 2022 15:18:20 -0700 Subject: [PATCH 26/67] Support mocking modules --- spec/spectator/mocks/mock_spec.cr | 197 ++++++++++++++++++++++++++++++ src/spectator/mocks/mock.cr | 7 +- 2 files changed, 203 insertions(+), 1 deletion(-) diff --git a/spec/spectator/mocks/mock_spec.cr b/spec/spectator/mocks/mock_spec.cr index 8835183..7d04fed 100644 --- a/spec/spectator/mocks/mock_spec.cr +++ b/spec/spectator/mocks/mock_spec.cr @@ -491,6 +491,203 @@ Spectator.describe Spectator::Mock do end end + context "with a module" do + module Thing + # `extend self` cannot be used. + # The Crystal compiler doesn't report the methods as class methods when doing so. + + def self.original_method + :original + end + + def self.default_method + :original + end + + def self.stubbed_method(_value = 42) + :original + end + end + + Spectator::Mock.define_subtype(:module, Thing, MockThing) do + stub def self.stubbed_method(_value = 42) + :stubbed + end + end + + let(mock) { MockThing } + + after { mock._spectator_clear_stubs } + + it "overrides an existing method" do + stub = Spectator::ValueStub.new(:original_method, :override) + expect { mock._spectator_define_stub(stub) }.to change { mock.original_method }.from(:original).to(:override) + end + + it "doesn't affect other methods" do + stub = Spectator::ValueStub.new(:stubbed_method, :override) + expect { mock._spectator_define_stub(stub) }.to_not change { mock.original_method } + end + + it "replaces an existing default stub" do + stub = Spectator::ValueStub.new(:default_method, :override) + expect { mock._spectator_define_stub(stub) }.to change { mock.default_method }.to(:override) + end + + it "replaces an existing stubbed method" do + stub = Spectator::ValueStub.new(:stubbed_method, :override) + expect { mock._spectator_define_stub(stub) }.to change { mock.stubbed_method }.to(:override) + end + + def restricted(thing : Thing.class) + thing.stubbed_method + end + + it "can be used in type restricted methods" do + expect(restricted(mock)).to eq(:stubbed) + end + + describe "._spectator_clear_stubs" do + before do + stub = Spectator::ValueStub.new(:original_method, :override) + mock._spectator_define_stub(stub) + end + + it "removes previously defined stubs" do + expect { mock._spectator_clear_stubs }.to change { mock.original_method }.from(:override).to(:original) + end + end + + describe "._spectator_calls" do + before { mock._spectator_clear_calls } + + # Retrieves symbolic names of methods called on a mock. + def called_method_names(mock) + mock._spectator_calls.map(&.method) + end + + it "stores calls to original methods" do + expect { mock.original_method }.to change { called_method_names(mock) }.from(%i[]).to(%i[original_method]) + end + + it "stores calls to default methods" do + expect { mock.default_method }.to change { called_method_names(mock) }.from(%i[]).to(%i[default_method]) + end + + it "stores calls to stubbed methods" do + expect { mock.stubbed_method }.to change { called_method_names(mock) }.from(%i[]).to(%i[stubbed_method]) + end + + it "stores multiple calls to the same stub" do + mock.stubbed_method + expect { mock.stubbed_method }.to change { called_method_names(mock) }.from(%i[stubbed_method]).to(%i[stubbed_method stubbed_method]) + end + + it "stores arguments for a call" do + mock.stubbed_method(5) + args = Spectator::Arguments.capture(5) + call = mock._spectator_calls.first + expect(call.arguments).to eq(args) + end + end + end + + context "with a mocked module included in a class" do + module Thing + def original_method + :original + end + + def default_method + :original + end + + def stubbed_method(_value = 42) + :original + end + end + + Spectator::Mock.define_subtype(:module, Thing, MockThing, default_method: :default) do + stub def stubbed_method(_value = 42) + :stubbed + end + end + + class IncludedMock + include MockThing + end + + let(mock) { IncludedMock.new } + + it "overrides an existing method" do + stub = Spectator::ValueStub.new(:original_method, :override) + expect { mock._spectator_define_stub(stub) }.to change { mock.original_method }.from(:original).to(:override) + end + + it "doesn't affect other methods" do + stub = Spectator::ValueStub.new(:stubbed_method, :override) + expect { mock._spectator_define_stub(stub) }.to_not change { mock.original_method } + end + + it "replaces an existing default stub" do + stub = Spectator::ValueStub.new(:default_method, :override) + expect { mock._spectator_define_stub(stub) }.to change { mock.default_method }.to(:override) + end + + it "replaces an existing stubbed method" do + stub = Spectator::ValueStub.new(:stubbed_method, :override) + expect { mock._spectator_define_stub(stub) }.to change { mock.stubbed_method }.to(:override) + end + + def restricted(thing : Thing.class) + thing.default_method + end + + describe "#_spectator_clear_stubs" do + before do + stub = Spectator::ValueStub.new(:original_method, :override) + mock._spectator_define_stub(stub) + end + + it "removes previously defined stubs" do + expect { mock._spectator_clear_stubs }.to change { mock.original_method }.from(:override).to(:original) + end + end + + describe "#_spectator_calls" do + before { mock._spectator_clear_calls } + + # Retrieves symbolic names of methods called on a mock. + def called_method_names(mock) + mock._spectator_calls.map(&.method) + end + + it "stores calls to original methods" do + expect { mock.original_method }.to change { called_method_names(mock) }.from(%i[]).to(%i[original_method]) + end + + it "stores calls to default methods" do + expect { mock.default_method }.to change { called_method_names(mock) }.from(%i[]).to(%i[default_method]) + end + + it "stores calls to stubbed methods" do + expect { mock.stubbed_method }.to change { called_method_names(mock) }.from(%i[]).to(%i[stubbed_method]) + end + + it "stores multiple calls to the same stub" do + mock.stubbed_method + expect { mock.stubbed_method }.to change { called_method_names(mock) }.from(%i[stubbed_method]).to(%i[stubbed_method stubbed_method]) + end + + it "stores arguments for a call" do + mock.stubbed_method(5) + args = Spectator::Arguments.capture(5) + call = mock._spectator_calls.first + expect(call.arguments).to eq(args) + end + end + end + context "with a method that uses NoReturn" do abstract class Thing abstract def oops : NoReturn diff --git a/src/spectator/mocks/mock.cr b/src/spectator/mocks/mock.cr index a5d6128..3c0eefe 100644 --- a/src/spectator/mocks/mock.cr +++ b/src/spectator/mocks/mock.cr @@ -36,7 +36,12 @@ module Spectator macro define_subtype(base, mocked_type, type_name, name = nil, **value_methods, &block) {% begin %} {% if name %}@[::Spectator::StubbedName({{name}})]{% end %} - {{base.id}} {{type_name.id}} < {{mocked_type.id}} + {% if base.id == :module.id %} + {{base.id}} {{type_name.id}} + include {{mocked_type.id}} + {% else %} + {{base.id}} {{type_name.id}} < {{mocked_type.id}} + {% end %} include ::Spectator::Mocked extend ::Spectator::StubbedType From fa99987780369a5da0183e1354a2381b054374f9 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sun, 18 Dec 2022 16:04:49 -0700 Subject: [PATCH 27/67] Support creating instances of mocked modules via class This is a bit of a hack. The `.new` method is added to the module, which creates an instance that includes the mocked module. No changes to the def_mock and new_mock methods are necessary. For some reason, infinite recursion occurs when calling `.new` on the class. To get around the issue for now, the internal method of allocation is used. That is, allocate + initialize. --- CHANGELOG.md | 3 + spec/spectator/dsl/mocks/mock_spec.cr | 258 ++++++++++++++++++++++++++ src/spectator/mocks/mock.cr | 23 +++ test.cr | 104 +++++++++++ 4 files changed, 388 insertions(+) create mode 100644 test.cr diff --git a/CHANGELOG.md b/CHANGELOG.md index 73ffadb..bf4bc1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added +- Added support for mock modules and types that include mocked modules. + ### Fixed - Fix macro logic to support free variables, 'self', and variants on stubbed methods. [#48](https://github.com/icy-arctic-fox/spectator/issues/48) - Fix method stubs used on methods that capture blocks. diff --git a/spec/spectator/dsl/mocks/mock_spec.cr b/spec/spectator/dsl/mocks/mock_spec.cr index db63dd9..05ef4c9 100644 --- a/spec/spectator/dsl/mocks/mock_spec.cr +++ b/spec/spectator/dsl/mocks/mock_spec.cr @@ -1027,4 +1027,262 @@ Spectator.describe "Mock DSL", :smoke do expect(fake.reference).to eq("reference") end end + + describe "mock module" do + module Dummy + # `extend self` cannot be used. + # The Crystal compiler doesn't report the methods as class methods when doing so. + + def self.abstract_method + :not_really_abstract + end + + def self.default_method + :original + end + + def self.args(arg) + arg + end + + def self.method1 + :original + end + + def self.reference + method1.to_s + end + end + + mock(Dummy) do + abstract_stub def self.abstract_method + :abstract + end + + stub def self.default_method + :default + end + end + + let(fake) { class_mock(Dummy) } + + it "raises on abstract stubs" do + expect { fake.abstract_method }.to raise_error(Spectator::UnexpectedMessage, /abstract_method/) + end + + it "can define default stubs" do + expect(fake.default_method).to eq(:default) + end + + it "can define new stubs" do + expect { allow(fake).to receive(:args).and_return(42) }.to change { fake.args(5) }.from(5).to(42) + end + + it "can override class method stubs" do + allow(fake).to receive(:method1).and_return(:override) + expect(fake.method1).to eq(:override) + end + + xit "can reference stubs", pending: "Default stub of module class methods always refer to original" do + allow(fake).to receive(:method1).and_return(:reference) + expect(fake.reference).to eq("reference") + end + end + + context "with a class including a mocked module" do + module Dummy + getter _spectator_invocations = [] of Symbol + + def method1 + @_spectator_invocations << :method1 + "original" + end + + def method2 : Symbol + @_spectator_invocations << :method2 + :original + end + + def method3(arg) + @_spectator_invocations << :method3 + arg + end + + def method4 : Symbol + @_spectator_invocations << :method4 + yield + end + + def method5 + @_spectator_invocations << :method5 + yield.to_i + end + + def method6 + @_spectator_invocations << :method6 + yield + end + + def method7(arg, *args, kwarg, **kwargs) + @_spectator_invocations << :method7 + {arg, args, kwarg, kwargs} + end + + def method8(arg, *args, kwarg, **kwargs) + @_spectator_invocations << :method8 + yield + {arg, args, kwarg, kwargs} + end + end + + # method1 stubbed via mock block + # method2 stubbed via keyword args + # method3 not stubbed (calls original) + # method4 stubbed via mock block (yields) + # method5 stubbed via keyword args (yields) + # method6 not stubbed (calls original and yields) + # method7 not stubbed (calls original) testing args + # method8 not stubbed (calls original and yields) testing args + mock(Dummy, method2: :stubbed, method5: 42) do + stub def method1 + "stubbed" + end + + stub def method4 : Symbol + yield + :block + end + end + + subject(fake) { mock(Dummy) } + + it "defines a subclass" do + expect(fake).to be_a(Dummy) + end + + it "defines stubs in the block" do + expect(fake.method1).to eq("stubbed") + end + + it "can stub methods defined in the block" do + stub = Spectator::ValueStub.new(:method1, "override") + expect { fake._spectator_define_stub(stub) }.to change { fake.method1 }.from("stubbed").to("override") + end + + it "defines stubs from keyword arguments" do + expect(fake.method2).to eq(:stubbed) + end + + it "can stub methods from keyword arguments" do + stub = Spectator::ValueStub.new(:method2, :override) + expect { fake._spectator_define_stub(stub) }.to change { fake.method2 }.from(:stubbed).to(:override) + end + + it "calls the original implementation for methods not provided a stub" do + expect(fake.method3(:xyz)).to eq(:xyz) + end + + it "can stub methods after declaration" do + stub = Spectator::ValueStub.new(:method3, :abc) + expect { fake._spectator_define_stub(stub) }.to change { fake.method3(:xyz) }.from(:xyz).to(:abc) + end + + it "defines stubs with yield in the block" do + expect(fake.method4 { :wrong }).to eq(:block) + end + + it "can stub methods with yield in the block" do + stub = Spectator::ValueStub.new(:method4, :override) + expect { fake._spectator_define_stub(stub) }.to change { fake.method4 { :wrong } }.from(:block).to(:override) + end + + it "defines stubs with yield from keyword arguments" do + expect(fake.method5 { :wrong }).to eq(42) + end + + it "can stub methods with yield from keyword arguments" do + stub = Spectator::ValueStub.new(:method5, 123) + expect { fake._spectator_define_stub(stub) }.to change { fake.method5 { "0" } }.from(42).to(123) + end + + it "can stub yielding methods after declaration" do + stub = Spectator::ValueStub.new(:method6, :abc) + expect { fake._spectator_define_stub(stub) }.to change { fake.method6 { :xyz } }.from(:xyz).to(:abc) + end + + it "handles arguments correctly" do + args1 = fake.method7(1, 2, 3, kwarg: 4, x: 5, y: 6, z: 7) + args2 = fake.method8(1, 2, 3, kwarg: 4, x: 5, y: 6, z: 7) { :block } + aggregate_failures do + expect(args1).to eq({1, {2, 3}, 4, {x: 5, y: 6, z: 7}}) + expect(args2).to eq({1, {2, 3}, 4, {x: 5, y: 6, z: 7}}) + end + end + + it "handles arguments correctly with stubs" do + stub1 = Spectator::ProcStub.new(:method7, args_proc) + stub2 = Spectator::ProcStub.new(:method8, args_proc) + fake._spectator_define_stub(stub1) + fake._spectator_define_stub(stub2) + args1 = fake.method7(1, 2, 3, kwarg: 4, x: 5, y: 6, z: 7) + args2 = fake.method8(1, 2, 3, kwarg: 4, x: 5, y: 6, z: 7) { :block } + aggregate_failures do + expect(args1).to eq({1, {2, 3}, 4, {x: 5, y: 6, z: 7}}) + expect(args2).to eq({1, {2, 3}, 4, {x: 5, y: 6, z: 7}}) + end + end + + it "compiles types without unions" do + aggregate_failures do + expect(fake.method1).to compile_as(String) + expect(fake.method2).to compile_as(Symbol) + expect(fake.method3(42)).to compile_as(Int32) + expect(fake.method4 { :foo }).to compile_as(Symbol) + expect(fake.method5 { "123" }).to compile_as(Int32) + expect(fake.method6 { "123" }).to compile_as(String) + end + end + + def restricted(thing : Dummy) + thing.method1 + end + + it "can be used in type restricted methods" do + expect(restricted(fake)).to eq("stubbed") + end + + it "does not call the original method when stubbed" do + fake.method1 + fake.method2 + fake.method3("foo") + fake.method4 { :foo } + fake.method5 { "42" } + fake.method6 { 42 } + fake.method7(1, 2, 3, kwarg: 4, x: 5, y: 6, z: 7) + fake.method8(1, 2, 3, kwarg: 4, x: 5, y: 6, z: 7) { :block } + expect(fake._spectator_invocations).to contain_exactly(:method3, :method6, :method7, :method8) + end + + # Cannot test unexpected messages - will not compile due to missing methods. + + describe "deferred default stubs" do + mock(Dummy) + + let(fake2) do + mock(Dummy, + method1: "stubbed", + method3: 123, + method4: :xyz) + end + + it "uses the keyword arguments as stubs" do + aggregate_failures do + expect(fake2.method1).to eq("stubbed") + expect(fake2.method2).to eq(:original) + expect(fake2.method3(42)).to eq(123) + expect(fake2.method4 { :foo }).to eq(:xyz) + end + end + end + end end diff --git a/src/spectator/mocks/mock.cr b/src/spectator/mocks/mock.cr index 3c0eefe..248d8c8 100644 --- a/src/spectator/mocks/mock.cr +++ b/src/spectator/mocks/mock.cr @@ -39,6 +39,29 @@ module Spectator {% if base.id == :module.id %} {{base.id}} {{type_name.id}} include {{mocked_type.id}} + + # Mock class that includes the mocked module {{mocked_type.id}} + {% if name %}@[::Spectator::StubbedName({{name}})]{% end %} + private class ClassIncludingMock{{type_name.id}} + include {{type_name.id}} + end + + # Returns a mock class that includes the mocked module {{mocked_type.id}}. + def self.new(*args, **kwargs) : ClassIncludingMock{{type_name.id}} + # FIXME: Creating the instance normally with `.new` causing infinite recursion. + inst = ClassIncludingMock{{type_name.id}}.allocate + inst.initialize(*args, **kwargs) + inst + end + + # Returns a mock class that includes the mocked module {{mocked_type.id}}. + def self.new(*args, **kwargs) : ClassIncludingMock{{type_name.id}} + # FIXME: Creating the instance normally with `.new` causing infinite recursion. + inst = ClassIncludingMock{{type_name.id}}.allocate + inst.initialize(*args, **kwargs) { |*yargs| yield *yargs } + inst + end + {% else %} {{base.id}} {{type_name.id}} < {{mocked_type.id}} {% end %} diff --git a/test.cr b/test.cr new file mode 100644 index 0000000..65cb051 --- /dev/null +++ b/test.cr @@ -0,0 +1,104 @@ +require "./src/spectator" + +module Thing + def self.original_method + :original + end + + def self.default_method + :original + end + + def self.stubbed_method(_value = 42) + :original + end + + macro finished + def self.debug + {% begin %}puts "Methods: ", {{@type.methods.map &.name.stringify}} of String{% end %} + {% begin %}puts "Class Methods: ", {{@type.class.methods.map &.name.stringify}} of String{% end %} + end + end +end + +Thing.debug + +# Spectator::Mock.define_subtype(:module, Thing, MockThing, default_method: :default) do +# stub def stubbed_method(_value = 42) +# :stubbed +# end +# end + +# Spectator.describe "Mock modules" do +# let(mock) { MockThing } + +# after { mock._spectator_clear_stubs } + +# it "overrides an existing method" do +# stub = Spectator::ValueStub.new(:original_method, :override) +# expect { mock._spectator_define_stub(stub) }.to change { mock.original_method }.from(:original).to(:override) +# end + +# it "doesn't affect other methods" do +# stub = Spectator::ValueStub.new(:stubbed_method, :override) +# expect { mock._spectator_define_stub(stub) }.to_not change { mock.original_method } +# end + +# it "replaces an existing default stub" do +# stub = Spectator::ValueStub.new(:default_method, :override) +# expect { mock._spectator_define_stub(stub) }.to change { mock.default_method }.to(:override) +# end + +# it "replaces an existing stubbed method" do +# stub = Spectator::ValueStub.new(:stubbed_method, :override) +# expect { mock._spectator_define_stub(stub) }.to change { mock.stubbed_method }.to(:override) +# end + +# def restricted(thing : Thing.class) +# thing.default_method +# end + +# describe "._spectator_clear_stubs" do +# before do +# stub = Spectator::ValueStub.new(:original_method, :override) +# mock._spectator_define_stub(stub) +# end + +# it "removes previously defined stubs" do +# expect { mock._spectator_clear_stubs }.to change { mock.original_method }.from(:override).to(:original) +# end +# end + +# describe "._spectator_calls" do +# before { mock._spectator_clear_calls } + +# # Retrieves symbolic names of methods called on a mock. +# def called_method_names(mock) +# mock._spectator_calls.map(&.method) +# end + +# it "stores calls to original methods" do +# expect { mock.original_method }.to change { called_method_names(mock) }.from(%i[]).to(%i[original_method]) +# end + +# it "stores calls to default methods" do +# expect { mock.default_method }.to change { called_method_names(mock) }.from(%i[]).to(%i[default_method]) +# end + +# it "stores calls to stubbed methods" do +# expect { mock.stubbed_method }.to change { called_method_names(mock) }.from(%i[]).to(%i[stubbed_method]) +# end + +# it "stores multiple calls to the same stub" do +# mock.stubbed_method +# expect { mock.stubbed_method }.to change { called_method_names(mock) }.from(%i[stubbed_method]).to(%i[stubbed_method stubbed_method]) +# end + +# it "stores arguments for a call" do +# mock.stubbed_method(5) +# args = Spectator::Arguments.capture(5) +# call = mock._spectator_calls.first +# expect(call.arguments).to eq(args) +# end +# end +# end From a3c55dfa4750cea0acf528e3d8eda4521dafebc4 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sun, 18 Dec 2022 18:52:08 -0700 Subject: [PATCH 28/67] Add tests for module mocks docs --- spec/docs/mocks_spec.cr | 84 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/spec/docs/mocks_spec.cr b/spec/docs/mocks_spec.cr index 5908787..353c35e 100644 --- a/spec/docs/mocks_spec.cr +++ b/spec/docs/mocks_spec.cr @@ -123,6 +123,90 @@ Spectator.describe "Mocks Docs" do end end + context "Mock Modules" do + module MyModule + def something + # ... + end + end + + describe "#something" do + # Define a mock for MyModule. + mock MyClass + + it "does something" do + # Use mock here. + end + end + + module MyFileUtils + def self.rm_rf(path) + # ... + end + end + + mock MyFileUtils + + it "deletes all of my files" do + utils = class_mock(MyFileUtils) + allow(utils).to receive(:rm_rf) + utils.rm_rf("/") + expect(utils).to have_received(:rm_rf).with("/") + end + + module MyFileUtils2 + extend self + + def rm_rf(path) + # ... + end + end + + mock(MyFileUtils2) do + # Define a default stub for the method. + stub def self.rm_rf(path) + # ... + end + end + + it "deletes all of my files part 2" do + utils = class_mock(MyFileUtils2) + allow(utils).to receive(:rm_rf) + utils.rm_rf("/") + expect(utils).to have_received(:rm_rf).with("/") + end + + module Runnable + def run + # ... + end + end + + mock Runnable + + specify do + runnable = mock(Runnable) # or new_mock(Runnable) + runnable.run + end + + module Runnable2 + abstract def command : String + + def run_one + "Running #{command}" + end + end + + mock Runnable2, command: "ls -l" + + specify do + runnable = mock(Runnable2) + expect(runnable.run_one).to eq("Running ls -l") + runnable = mock(Runnable2, command: "echo foo") + expect(runnable.run_one).to eq("Running echo foo") + end + end + context "Injecting Mocks" do struct MyStruct def something From 8f80b10fc1d46d2f2e9d6676369b6e679f57a115 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sun, 18 Dec 2022 19:04:50 -0700 Subject: [PATCH 29/67] Support injecting mock functionality into modules Add mock registry for a single module. --- spec/docs/mocks_spec.cr | 19 ++++++++++++ src/spectator/mocks/mock.cr | 3 +- src/spectator/mocks/mock_registry.cr | 43 ++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 src/spectator/mocks/mock_registry.cr diff --git a/spec/docs/mocks_spec.cr b/spec/docs/mocks_spec.cr index 353c35e..58c16f8 100644 --- a/spec/docs/mocks_spec.cr +++ b/spec/docs/mocks_spec.cr @@ -205,6 +205,25 @@ Spectator.describe "Mocks Docs" do runnable = mock(Runnable2, command: "echo foo") expect(runnable.run_one).to eq("Running echo foo") end + + context "Injecting Mocks" do + module MyFileUtils + def self.rm_rf(path) + true + end + end + + inject_mock MyFileUtils do + stub def self.rm_rf(path) + "Simulating deletion of #{path}" + false + end + end + + specify do + expect(MyFileUtils.rm_rf("/")).to be_false + end + end end context "Injecting Mocks" do diff --git a/src/spectator/mocks/mock.cr b/src/spectator/mocks/mock.cr index 248d8c8..87e8e23 100644 --- a/src/spectator/mocks/mock.cr +++ b/src/spectator/mocks/mock.cr @@ -1,5 +1,6 @@ require "./method_call" require "./mocked" +require "./mock_registry" require "./reference_mock_registry" require "./stub" require "./stubbed_name" @@ -157,7 +158,7 @@ module Spectator {% elsif base == :struct %} @@_spectator_mock_registry = ::Spectator::ValueMockRegistry(self).new {% else %} - {% raise "Unsupported base type #{base} for injecting mock" %} + @@_spectator_mock_registry = ::Spectator::MockRegistry.new {% end %} private class_getter _spectator_stubs : ::Array(::Spectator::Stub) = [] of ::Spectator::Stub diff --git a/src/spectator/mocks/mock_registry.cr b/src/spectator/mocks/mock_registry.cr new file mode 100644 index 0000000..29390c6 --- /dev/null +++ b/src/spectator/mocks/mock_registry.cr @@ -0,0 +1,43 @@ +require "./mock_registry_entry" +require "./stub" + +module Spectator + # Stores collections of stubs for mocked types. + # + # This type is intended for all mocked modules that have functionality "injected." + # That is, the type itself has mock functionality bolted on. + # Adding instance members should be avoided, for instance, it could mess up serialization. + class MockRegistry + @entry : MockRegistryEntry? + + # Retrieves all stubs. + def [](_object = nil) + @entry.not_nil! + end + + # Retrieves all stubs. + def []?(_object = nil) + @entry + end + + # Retrieves all stubs. + # + # Yields to the block on the first retrieval. + # This allows a mock to populate the registry with initial stubs. + def fetch(object : Reference, & : -> Array(Stub)) + entry = @entry + if entry.nil? + entry = MockRegistryEntry.new + entry.stubs = yield + @entry = entry + else + entry + end + end + + # Clears all stubs defined for a mocked object. + def delete(object : Reference) : Nil + @entry = nil + end + end +end From feaf1c601538c45a6f97bbceec720554d8575b20 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sun, 18 Dec 2022 19:15:25 -0700 Subject: [PATCH 30/67] Bump version to 0.11.5 --- CHANGELOG.md | 6 ++++-- shard.yml | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf4bc1f..40813e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [0.11.5] - 2022-12-18 ### Added - Added support for mock modules and types that include mocked modules. @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Remove unnecessary redefinitions of methods when adding stub functionality to a type. - Allow metadata to be stored as nil to reduce overhead when tracking nodes without tags. - Use normal equality (==) instead of case-equality (===) with proc arguments in stubs. +- Change stub value cast logic to avoid compiler bug. [#80](https://gitlab.com/arctic-fox/spectator/-/issues/80) ## [0.11.4] - 2022-11-27 ### Added @@ -432,7 +433,8 @@ This has been changed so that it compiles and raises an error at runtime with a First version ready for public use. -[Unreleased]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.4...master +[Unreleased]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.5...master +[0.11.5]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.4...v0.11.5 [0.11.4]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.3...v0.11.4 [0.11.3]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.2...v0.11.3 [0.11.2]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.1...v0.11.2 diff --git a/shard.yml b/shard.yml index 15c9a65..a41bc90 100644 --- a/shard.yml +++ b/shard.yml @@ -1,5 +1,5 @@ name: spectator -version: 0.11.4 +version: 0.11.5 description: | Feature-rich testing framework for Crystal inspired by RSpec. From 7620f58fb886cef889b456ca4c6e5cd2f2a69691 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Mon, 19 Dec 2022 02:31:12 -0700 Subject: [PATCH 31/67] Test file, please ignore --- .gitignore | 2 ++ test.cr | 104 ----------------------------------------------------- 2 files changed, 2 insertions(+), 104 deletions(-) delete mode 100644 test.cr diff --git a/.gitignore b/.gitignore index c4166ba..f76b510 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,5 @@ # Ignore JUnit output output.xml + +/test.cr diff --git a/test.cr b/test.cr deleted file mode 100644 index 65cb051..0000000 --- a/test.cr +++ /dev/null @@ -1,104 +0,0 @@ -require "./src/spectator" - -module Thing - def self.original_method - :original - end - - def self.default_method - :original - end - - def self.stubbed_method(_value = 42) - :original - end - - macro finished - def self.debug - {% begin %}puts "Methods: ", {{@type.methods.map &.name.stringify}} of String{% end %} - {% begin %}puts "Class Methods: ", {{@type.class.methods.map &.name.stringify}} of String{% end %} - end - end -end - -Thing.debug - -# Spectator::Mock.define_subtype(:module, Thing, MockThing, default_method: :default) do -# stub def stubbed_method(_value = 42) -# :stubbed -# end -# end - -# Spectator.describe "Mock modules" do -# let(mock) { MockThing } - -# after { mock._spectator_clear_stubs } - -# it "overrides an existing method" do -# stub = Spectator::ValueStub.new(:original_method, :override) -# expect { mock._spectator_define_stub(stub) }.to change { mock.original_method }.from(:original).to(:override) -# end - -# it "doesn't affect other methods" do -# stub = Spectator::ValueStub.new(:stubbed_method, :override) -# expect { mock._spectator_define_stub(stub) }.to_not change { mock.original_method } -# end - -# it "replaces an existing default stub" do -# stub = Spectator::ValueStub.new(:default_method, :override) -# expect { mock._spectator_define_stub(stub) }.to change { mock.default_method }.to(:override) -# end - -# it "replaces an existing stubbed method" do -# stub = Spectator::ValueStub.new(:stubbed_method, :override) -# expect { mock._spectator_define_stub(stub) }.to change { mock.stubbed_method }.to(:override) -# end - -# def restricted(thing : Thing.class) -# thing.default_method -# end - -# describe "._spectator_clear_stubs" do -# before do -# stub = Spectator::ValueStub.new(:original_method, :override) -# mock._spectator_define_stub(stub) -# end - -# it "removes previously defined stubs" do -# expect { mock._spectator_clear_stubs }.to change { mock.original_method }.from(:override).to(:original) -# end -# end - -# describe "._spectator_calls" do -# before { mock._spectator_clear_calls } - -# # Retrieves symbolic names of methods called on a mock. -# def called_method_names(mock) -# mock._spectator_calls.map(&.method) -# end - -# it "stores calls to original methods" do -# expect { mock.original_method }.to change { called_method_names(mock) }.from(%i[]).to(%i[original_method]) -# end - -# it "stores calls to default methods" do -# expect { mock.default_method }.to change { called_method_names(mock) }.from(%i[]).to(%i[default_method]) -# end - -# it "stores calls to stubbed methods" do -# expect { mock.stubbed_method }.to change { called_method_names(mock) }.from(%i[]).to(%i[stubbed_method]) -# end - -# it "stores multiple calls to the same stub" do -# mock.stubbed_method -# expect { mock.stubbed_method }.to change { called_method_names(mock) }.from(%i[stubbed_method]).to(%i[stubbed_method stubbed_method]) -# end - -# it "stores arguments for a call" do -# mock.stubbed_method(5) -# args = Spectator::Arguments.capture(5) -# call = mock._spectator_calls.first -# expect(call.arguments).to eq(args) -# end -# end -# end From 0f8c46d6efe36e34b903c05b4e0a292a96829f72 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Mon, 19 Dec 2022 21:29:21 -0700 Subject: [PATCH 32/67] Support casting types with expect statements --- CHANGELOG.md | 4 +++ spec/features/expect_type_spec.cr | 36 +++++++++++++++++++++++++ src/spectator/expectation.cr | 45 +++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) create mode 100644 spec/features/expect_type_spec.cr diff --git a/CHANGELOG.md b/CHANGELOG.md index 40813e7..9a79e5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] +### Added +- Added ability to cast types using the return value from expect statements with a type matcher. + ## [0.11.5] - 2022-12-18 ### Added - Added support for mock modules and types that include mocked modules. diff --git a/spec/features/expect_type_spec.cr b/spec/features/expect_type_spec.cr new file mode 100644 index 0000000..6991a8a --- /dev/null +++ b/spec/features/expect_type_spec.cr @@ -0,0 +1,36 @@ +require "../spec_helper" + +Spectator.describe "Expect Type" do + it "ensures a type is cast" do + value = 42.as(String | Int32) + expect(value).to be_a(String | Int32) + expect(value).to compile_as(String | Int32) + value = expect(value).to be_a(Int32) + expect(value).to eq(42) + expect(value).to be_a(Int32) + expect(value).to compile_as(Int32) + expect(value).to_not respond_to(:downcase) + end + + it "ensures a type is not nil" do + value = 42.as(Int32?) + expect(value).to be_a(Int32?) + expect(value).to compile_as(Int32?) + value = expect(value).to_not be_nil + expect(value).to eq(42) + expect(value).to be_a(Int32) + expect(value).to compile_as(Int32) + expect { value.not_nil! }.to_not raise_error(NilAssertionError) + end + + it "removes types from a union" do + value = 42.as(String | Int32) + expect(value).to be_a(String | Int32) + expect(value).to compile_as(String | Int32) + value = expect(value).to_not be_a(String) + expect(value).to eq(42) + expect(value).to be_a(Int32) + expect(value).to compile_as(Int32) + expect(value).to_not respond_to(:downcase) + end +end diff --git a/src/spectator/expectation.cr b/src/spectator/expectation.cr index 780e299..79d8473 100644 --- a/src/spectator/expectation.cr +++ b/src/spectator/expectation.cr @@ -114,6 +114,21 @@ module Spectator report(match_data, message) end + # Asserts that some criteria defined by the matcher is satisfied. + # Allows a custom message to be used. + # Returns the expected value cast as the expected type, if the matcher is satisfied. + def to(matcher : Matchers::TypeMatcher(U), message = nil) forall U + match_data = matcher.match(@expression) + value = @expression.value + if report(match_data, message) + return value if value.is_a?(U) + + raise "Spectator bug: expected value should have cast to #{U}" + else + raise TypeCastError.new("#{@expression.label} is expected to be a #{U}, but was actually #{value.class}") + end + end + # Asserts that a method is not called before the example completes. @[AlwaysInline] def to_not(stub : Stub, message = nil) : Nil @@ -136,6 +151,36 @@ module Spectator report(match_data, message) end + # Asserts that some criteria defined by the matcher is not satisfied. + # Allows a custom message to be used. + # Returns the expected value cast without the unexpected type, if the matcher is satisfied. + def to_not(matcher : Matchers::TypeMatcher(U), message = nil) forall U + match_data = matcher.negated_match(@expression) + value = @expression.value + if report(match_data, message) + return value unless value.is_a?(U) + + raise "Spectator bug: expected value should not be #{U}" + else + raise TypeCastError.new("#{@expression.label} is not expected to be a #{U}, but was actually #{value.class}") + end + end + + # Asserts that some criteria defined by the matcher is not satisfied. + # Allows a custom message to be used. + # Returns the expected value cast as a non-nillable type, if the matcher is satisfied. + def to_not(matcher : Matchers::NilMatcher, message = nil) + match_data = matcher.negated_match(@expression) + if report(match_data, message) + value = @expression.value + return value unless value.nil? + + raise "Spectator bug: expected value should not be nil" + else + raise NilAssertionError.new("#{@expression.label} is not expected to be nil.") + end + end + # :ditto: @[AlwaysInline] def not_to(matcher, message = nil) : Nil From faff2933e6d83e087529576073accf4a3af74541 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Mon, 19 Dec 2022 22:15:53 -0700 Subject: [PATCH 33/67] Only capture splat if it has a name --- CHANGELOG.md | 3 +++ src/spectator/mocks/stubbable.cr | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a79e5e..6881601 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Added ability to cast types using the return value from expect statements with a type matcher. +### Fixed +- Fix invalid syntax (unterminated call) when recording calls to stubs with an un-named splat. + ## [0.11.5] - 2022-12-18 ### Added - Added support for mock modules and types that include mocked modules. diff --git a/src/spectator/mocks/stubbable.cr b/src/spectator/mocks/stubbable.cr index 48f6ac5..119111d 100644 --- a/src/spectator/mocks/stubbable.cr +++ b/src/spectator/mocks/stubbable.cr @@ -180,7 +180,7 @@ module Spectator ::NamedTuple.new( {% for arg, i in method.args %}{% if !method.splat_index || i < method.splat_index %}{{arg.internal_name.stringify}}: {{arg.internal_name}}, {% end %}{% end %} ), - {% if method.splat_index && (splat = method.args[method.splat_index].internal_name) %}{{splat.symbolize}}, {{splat}},{% end %} + {% if method.splat_index && !(splat = method.args[method.splat_index].internal_name).empty? %}{{splat.symbolize}}, {{splat}},{% end %} ::NamedTuple.new( {% for arg, i in method.args %}{% if method.splat_index && i > method.splat_index %}{{arg.internal_name.stringify}}: {{arg.internal_name}}, {% end %}{% end %} ).merge({{method.double_splat}}) @@ -332,7 +332,7 @@ module Spectator ::NamedTuple.new( {% for arg, i in method.args %}{% if !method.splat_index || i < method.splat_index %}{{arg.internal_name.stringify}}: {{arg.internal_name}}, {% end %}{% end %} ), - {% if method.splat_index && (splat = method.args[method.splat_index].internal_name) %}{{splat.symbolize}}, {{splat}},{% end %} + {% if method.splat_index && !(splat = method.args[method.splat_index].internal_name).empty? %}{{splat.symbolize}}, {{splat}},{% end %} ::NamedTuple.new( {% for arg, i in method.args %}{% if method.splat_index && i > method.splat_index %}{{arg.internal_name.stringify}}: {{arg.internal_name}}, {% end %}{% end %} ).merge({{method.double_splat}}) From acf810553aa2c0c1704460c8b99260aed11e3515 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Mon, 19 Dec 2022 22:27:58 -0700 Subject: [PATCH 34/67] Use location of the 'should' keyword for their expectation --- CHANGELOG.md | 3 +++ src/spectator/should.cr | 28 ++++++++++++++++------------ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6881601..ddcdc86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix invalid syntax (unterminated call) when recording calls to stubs with an un-named splat. +### Changed +- Expectations using 'should' syntax report file and line where the 'should' keyword is instead of the test start. + ## [0.11.5] - 2022-12-18 ### Added - Added support for mock modules and types that include mocked modules. diff --git a/src/spectator/should.cr b/src/spectator/should.cr index eb0733f..d444c00 100644 --- a/src/spectator/should.cr +++ b/src/spectator/should.cr @@ -22,51 +22,55 @@ class Object # ``` # require "spectator/should" # ``` - def should(matcher, message = nil) + def should(matcher, message = nil, *, _file = __FILE__, _line = __LINE__) actual = ::Spectator::Value.new(self) + location = ::Spectator::Location.new(_file, _line) match_data = matcher.match(actual) - expectation = ::Spectator::Expectation.new(match_data, message: message) + expectation = ::Spectator::Expectation.new(match_data, location, message) ::Spectator::Harness.current.report(expectation) end # Works the same as `#should` except the condition is inverted. # When `#should` succeeds, this method will fail, and vice-versa. - def should_not(matcher, message = nil) + def should_not(matcher, message = nil, *, _file = __FILE__, _line = __LINE__) actual = ::Spectator::Value.new(self) + location = ::Spectator::Location.new(_file, _line) match_data = matcher.negated_match(actual) - expectation = ::Spectator::Expectation.new(match_data, message: message) + expectation = ::Spectator::Expectation.new(match_data, location, message) ::Spectator::Harness.current.report(expectation) end # Works the same as `#should` except that the condition check is postponed. # The expectation is checked after the example finishes and all hooks have run. - def should_eventually(matcher, message = nil) - ::Spectator::Harness.current.defer { should(matcher, message) } + def should_eventually(matcher, message = nil, *, _file = __FILE__, _line = __LINE__) + ::Spectator::Harness.current.defer { should(matcher, message, _file: _file, _line: _line) } end # Works the same as `#should_not` except that the condition check is postponed. # The expectation is checked after the example finishes and all hooks have run. - def should_never(matcher, message = nil) - ::Spectator::Harness.current.defer { should_not(matcher, message) } + def should_never(matcher, message = nil, *, _file = __FILE__, _line = __LINE__) + ::Spectator::Harness.current.defer { should_not(matcher, message, _file: _file, _line: _line) } end end struct Proc(*T, R) # Extension method to create an expectation for a block of code (proc). # Depending on the matcher, the proc may be executed multiple times. - def should(matcher, message = nil) + def should(matcher, message = nil, *, _file = __FILE__, _line = __LINE__) actual = ::Spectator::Block.new(self) + location = ::Spectator::Location.new(_file, _line) match_data = matcher.match(actual) - expectation = ::Spectator::Expectation.new(match_data, message: message) + expectation = ::Spectator::Expectation.new(match_data, location, message) ::Spectator::Harness.current.report(expectation) end # Works the same as `#should` except the condition is inverted. # When `#should` succeeds, this method will fail, and vice-versa. - def should_not(matcher, message = nil) + def should_not(matcher, message = nil, *, _file = __FILE__, _line = __LINE__) actual = ::Spectator::Block.new(self) + location = ::Spectator::Location.new(_file, _line) match_data = matcher.negated_match(actual) - expectation = ::Spectator::Expectation.new(match_data, message: message) + expectation = ::Spectator::Expectation.new(match_data, location, message) ::Spectator::Harness.current.report(expectation) end end From c4bcf54b987456b4be3377dfa33dee8af8f97032 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Mon, 19 Dec 2022 22:40:55 -0700 Subject: [PATCH 35/67] Support casting types with should statements --- CHANGELOG.md | 2 +- spec/features/expect_type_spec.cr | 92 +++++++++++++++++++++---------- src/spectator/should.cr | 51 +++++++++++++++++ 3 files changed, 115 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddcdc86..72d1db9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added -- Added ability to cast types using the return value from expect statements with a type matcher. +- Added ability to cast types using the return value from expect/should statements with a type matcher. ### Fixed - Fix invalid syntax (unterminated call) when recording calls to stubs with an un-named splat. diff --git a/spec/features/expect_type_spec.cr b/spec/features/expect_type_spec.cr index 6991a8a..6ed9949 100644 --- a/spec/features/expect_type_spec.cr +++ b/spec/features/expect_type_spec.cr @@ -1,36 +1,70 @@ require "../spec_helper" -Spectator.describe "Expect Type" do - it "ensures a type is cast" do - value = 42.as(String | Int32) - expect(value).to be_a(String | Int32) - expect(value).to compile_as(String | Int32) - value = expect(value).to be_a(Int32) - expect(value).to eq(42) - expect(value).to be_a(Int32) - expect(value).to compile_as(Int32) - expect(value).to_not respond_to(:downcase) +Spectator.describe "Expect Type", :smoke do + context "with expect syntax" do + it "ensures a type is cast" do + value = 42.as(String | Int32) + expect(value).to be_a(String | Int32) + expect(value).to compile_as(String | Int32) + value = expect(value).to be_a(Int32) + expect(value).to eq(42) + expect(value).to be_a(Int32) + expect(value).to compile_as(Int32) + expect(value).to_not respond_to(:downcase) + end + + it "ensures a type is not nil" do + value = 42.as(Int32?) + expect(value).to be_a(Int32?) + expect(value).to compile_as(Int32?) + value = expect(value).to_not be_nil + expect(value).to eq(42) + expect(value).to be_a(Int32) + expect(value).to compile_as(Int32) + expect { value.not_nil! }.to_not raise_error(NilAssertionError) + end + + it "removes types from a union" do + value = 42.as(String | Int32) + expect(value).to be_a(String | Int32) + expect(value).to compile_as(String | Int32) + value = expect(value).to_not be_a(String) + expect(value).to eq(42) + expect(value).to be_a(Int32) + expect(value).to compile_as(Int32) + expect(value).to_not respond_to(:downcase) + end end - it "ensures a type is not nil" do - value = 42.as(Int32?) - expect(value).to be_a(Int32?) - expect(value).to compile_as(Int32?) - value = expect(value).to_not be_nil - expect(value).to eq(42) - expect(value).to be_a(Int32) - expect(value).to compile_as(Int32) - expect { value.not_nil! }.to_not raise_error(NilAssertionError) - end + context "with should syntax" do + it "ensures a type is cast" do + value = 42.as(String | Int32) + value.should be_a(String | Int32) + value = value.should be_a(Int32) + value.should eq(42) + value.should be_a(Int32) + value.should compile_as(Int32) + value.should_not respond_to(:downcase) + end - it "removes types from a union" do - value = 42.as(String | Int32) - expect(value).to be_a(String | Int32) - expect(value).to compile_as(String | Int32) - value = expect(value).to_not be_a(String) - expect(value).to eq(42) - expect(value).to be_a(Int32) - expect(value).to compile_as(Int32) - expect(value).to_not respond_to(:downcase) + it "ensures a type is not nil" do + value = 42.as(Int32?) + value.should be_a(Int32?) + value = value.should_not be_nil + value.should eq(42) + value.should be_a(Int32) + value.should compile_as(Int32) + expect { value.not_nil! }.to_not raise_error(NilAssertionError) + end + + it "removes types from a union" do + value = 42.as(String | Int32) + value.should be_a(String | Int32) + value = value.should_not be_a(String) + value.should eq(42) + value.should be_a(Int32) + value.should compile_as(Int32) + value.should_not respond_to(:downcase) + end end end diff --git a/src/spectator/should.cr b/src/spectator/should.cr index d444c00..f0fe075 100644 --- a/src/spectator/should.cr +++ b/src/spectator/should.cr @@ -30,6 +30,23 @@ class Object ::Spectator::Harness.current.report(expectation) end + # Asserts that some criteria defined by the matcher is satisfied. + # Allows a custom message to be used. + # Returns the expected value cast as the expected type, if the matcher is satisfied. + def should(matcher : ::Spectator::Matchers::TypeMatcher(U), message = nil, *, _file = __FILE__, _line = __LINE__) forall U + actual = ::Spectator::Value.new(self) + location = ::Spectator::Location.new(_file, _line) + match_data = matcher.match(actual) + expectation = ::Spectator::Expectation.new(match_data, location, message) + if ::Spectator::Harness.current.report(expectation) + return self if self.is_a?(U) + + raise "Spectator bug: expected value should have cast to #{U}" + else + raise TypeCastError.new("Expected value should be a #{U}, but was actually #{self.class}") + end + end + # Works the same as `#should` except the condition is inverted. # When `#should` succeeds, this method will fail, and vice-versa. def should_not(matcher, message = nil, *, _file = __FILE__, _line = __LINE__) @@ -40,6 +57,40 @@ class Object ::Spectator::Harness.current.report(expectation) end + # Asserts that some criteria defined by the matcher is not satisfied. + # Allows a custom message to be used. + # Returns the expected value cast without the unexpected type, if the matcher is satisfied. + def should_not(matcher : ::Spectator::Matchers::TypeMatcher(U), message = nil, *, _file = __FILE__, _line = __LINE__) forall U + actual = ::Spectator::Value.new(self) + location = ::Spectator::Location.new(_file, _line) + match_data = matcher.negated_match(actual) + expectation = ::Spectator::Expectation.new(match_data, location, message) + if ::Spectator::Harness.current.report(expectation) + return self unless self.is_a?(U) + + raise "Spectator bug: expected value should not be #{U}" + else + raise TypeCastError.new("Expected value is not expected to be a #{U}, but was actually #{self.class}") + end + end + + # Asserts that some criteria defined by the matcher is not satisfied. + # Allows a custom message to be used. + # Returns the expected value cast as a non-nillable type, if the matcher is satisfied. + def should_not(matcher : ::Spectator::Matchers::NilMatcher, message = nil, *, _file = __FILE__, _line = __LINE__) + actual = ::Spectator::Value.new(self) + location = ::Spectator::Location.new(_file, _line) + match_data = matcher.negated_match(actual) + expectation = ::Spectator::Expectation.new(match_data, location, message) + if ::Spectator::Harness.current.report(expectation) + return self unless self.nil? + + raise "Spectator bug: expected value should not be nil" + else + raise NilAssertionError.new("Expected value should not be nil.") + end + end + # Works the same as `#should` except that the condition check is postponed. # The expectation is checked after the example finishes and all hooks have run. def should_eventually(matcher, message = nil, *, _file = __FILE__, _line = __LINE__) From b8901f522afc78ec9fe055f51993cc2942f5b7c2 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 20 Dec 2022 20:11:09 -0700 Subject: [PATCH 36/67] Remove unnecessary cast --- src/spectator/example.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spectator/example.cr b/src/spectator/example.cr index 67483e0..04d69c9 100644 --- a/src/spectator/example.cr +++ b/src/spectator/example.cr @@ -118,7 +118,7 @@ module Spectator begin @result = Harness.run do - if proc = @name_proc.as?(Proc(Example, String)) + if proc = @name_proc self.name = proc.call(self) end From 30602663fe87399e8572a90ca7464fc174fb4d31 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 20 Dec 2022 20:12:58 -0700 Subject: [PATCH 37/67] Add tests for interpolated labels The context label test intentionally fails. This functionality still needs to be implemented. --- spec/features/interpolated_label_spec.cr | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 spec/features/interpolated_label_spec.cr diff --git a/spec/features/interpolated_label_spec.cr b/spec/features/interpolated_label_spec.cr new file mode 100644 index 0000000..9c78c15 --- /dev/null +++ b/spec/features/interpolated_label_spec.cr @@ -0,0 +1,22 @@ +require "../spec_helper" + +Spectator.describe "Interpolated Label" do + let(foo) { "example" } + let(bar) { "context" } + + it "interpolates #{foo} labels" do |example| + expect(example.name).to eq("interpolates example labels") + end + + context "within a \#{bar}" do + let(foo) { "multiple" } + + it "interpolates context labels" do |example| + expect(example.group.name).to eq("within a context") + end + + it "interpolates #{foo} levels" do |example| + expect(example.name).to eq("interpolates multiple levels") + end + end +end From 8c3900adcbde9429559a35836e5726896d6513c1 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 20 Dec 2022 20:32:40 -0700 Subject: [PATCH 38/67] Add support for interpolation in context names --- CHANGELOG.md | 1 + spec/features/interpolated_label_spec.cr | 2 +- src/spectator/context.cr | 5 +++++ src/spectator/dsl/groups.cr | 2 +- 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72d1db9..fb7b645 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added - Added ability to cast types using the return value from expect/should statements with a type matcher. +- Added support for string interpolation in context names/labels. ### Fixed - Fix invalid syntax (unterminated call) when recording calls to stubs with an un-named splat. diff --git a/spec/features/interpolated_label_spec.cr b/spec/features/interpolated_label_spec.cr index 9c78c15..5506a1f 100644 --- a/spec/features/interpolated_label_spec.cr +++ b/spec/features/interpolated_label_spec.cr @@ -8,7 +8,7 @@ Spectator.describe "Interpolated Label" do expect(example.name).to eq("interpolates example labels") end - context "within a \#{bar}" do + context "within a #{bar}" do let(foo) { "multiple" } it "interpolates context labels" do |example| diff --git a/src/spectator/context.cr b/src/spectator/context.cr index 7fc126b..b9f532a 100644 --- a/src/spectator/context.cr +++ b/src/spectator/context.cr @@ -4,6 +4,11 @@ # This type is intentionally outside the `Spectator` module. # The reason for this is to prevent name collision when using the DSL to define a spec. abstract class SpectatorContext + # Evaluates the contents of a block within the scope of the context. + def eval + with self yield + end + # Produces a dummy string to represent the context as a string. # This prevents the default behavior, which normally stringifies instance variables. # Due to the sheer amount of types Spectator can create diff --git a/src/spectator/dsl/groups.cr b/src/spectator/dsl/groups.cr index 0e7b47b..f595791 100644 --- a/src/spectator/dsl/groups.cr +++ b/src/spectator/dsl/groups.cr @@ -137,7 +137,7 @@ module Spectator::DSL what.is_a?(NilLiteral) %} {{what}} {% elsif what.is_a?(StringInterpolation) %} - {% raise "String interpolation isn't supported for example group names" %} + {{@type.name}}.new.eval { {{what}} } {% else %} {{what.stringify}} {% end %} From d46698d81a444d192633bd599beb2e8bf07c53bb Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 20 Dec 2022 20:43:47 -0700 Subject: [PATCH 39/67] Use separate context for example name interpolation This simplifies some code. --- CHANGELOG.md | 1 + src/spectator/dsl/examples.cr | 4 +--- src/spectator/example.cr | 24 ------------------------ src/spectator/example_builder.cr | 11 +---------- 4 files changed, 3 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb7b645..dfbe8c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Expectations using 'should' syntax report file and line where the 'should' keyword is instead of the test start. +- String interpolation for example names/labels uses a separate context than the one used by the test. ## [0.11.5] - 2022-12-18 ### Added diff --git a/src/spectator/dsl/examples.cr b/src/spectator/dsl/examples.cr index 703f2d3..4524001 100644 --- a/src/spectator/dsl/examples.cr +++ b/src/spectator/dsl/examples.cr @@ -125,9 +125,7 @@ module Spectator::DSL {% if what.is_a?(StringLiteral) || what.is_a?(NilLiteral) %} {{what}} {% elsif what.is_a?(StringInterpolation) %} - ->(example : ::Spectator::Example) do - example.with_context(\{{@type.name}}) { {{what}} } - end + {{@type.name}}.new.eval { {{what}} } {% else %} {{what.stringify}} {% end %} diff --git a/src/spectator/example.cr b/src/spectator/example.cr index 04d69c9..0c20ccb 100644 --- a/src/spectator/example.cr +++ b/src/spectator/example.cr @@ -28,8 +28,6 @@ module Spectator # Is pending if the example hasn't run. getter result : Result = PendingResult.new("Example not run") - @name_proc : Proc(Example, String)? - # Creates the example. # An instance to run the test code in is given by *context*. # The *entrypoint* defines the test code (typically inside *context*). @@ -47,24 +45,6 @@ module Spectator group << self if group end - # Creates the example. - # An instance to run the test code in is given by *context*. - # The *entrypoint* defines the test code (typically inside *context*). - # The *name* describes the purpose of the example. - # It can be a proc to be evaluated in the context of the example. - # The *location* tracks where the example exists in source code. - # The example will be assigned to *group* if it is provided. - # A set of *metadata* can be used for filtering and modifying example behavior. - # Note: The metadata will not be merged with the parent metadata. - def initialize(@context : Context, @entrypoint : self ->, - @name_proc : Example -> String, location : Location? = nil, - @group : ExampleGroup? = nil, metadata = nil) - super(nil, location, metadata) - - # Ensure group is linked. - group << self if group - end - # Creates a dynamic example. # A block provided to this method will be called as-if it were the test code for the example. # The block will be given this example instance as an argument. @@ -118,10 +98,6 @@ module Spectator begin @result = Harness.run do - if proc = @name_proc - self.name = proc.call(self) - end - @group.try(&.call_before_all) if (parent = @group) parent.call_around_each(procsy).call diff --git a/src/spectator/example_builder.cr b/src/spectator/example_builder.cr index 23398d2..ae5866a 100644 --- a/src/spectator/example_builder.cr +++ b/src/spectator/example_builder.cr @@ -8,7 +8,7 @@ module Spectator # Constructs examples. # Call `#build` to produce an `Example`. class ExampleBuilder < NodeBuilder - @name : Proc(Example, String) | String? + @name : String? # Creates the builder. # A proc provided by *context_builder* is used to create a unique `Context` for each example produced by `#build`. @@ -18,15 +18,6 @@ module Spectator @name : String? = nil, @location : Location? = nil, @metadata : Metadata? = nil) end - # Creates the builder. - # A proc provided by *context_builder* is used to create a unique `Context` for each example produced by `#build`. - # The *entrypoint* indicates the proc used to invoke the test code in the example. - # The *name* is an interpolated string that runs in the context of the example. - # *location*, and *metadata* will be applied to the `Example` produced by `#build`. - def initialize(@context_builder : -> Context, @entrypoint : Example ->, - @name : Example -> String, @location : Location? = nil, @metadata : Metadata? = nil) - end - # Constructs an example with previously defined attributes and context. # The *parent* is an already constructed example group to nest the new example under. # It can be nil if the new example won't have a parent. From 4a0bfc1cb255002ecb0a92dddd8e9eb32e00e697 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 20 Dec 2022 20:52:01 -0700 Subject: [PATCH 40/67] Add smoke tag --- spec/features/interpolated_label_spec.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/interpolated_label_spec.cr b/spec/features/interpolated_label_spec.cr index 5506a1f..add8e93 100644 --- a/spec/features/interpolated_label_spec.cr +++ b/spec/features/interpolated_label_spec.cr @@ -1,6 +1,6 @@ require "../spec_helper" -Spectator.describe "Interpolated Label" do +Spectator.describe "Interpolated Label", :smoke do let(foo) { "example" } let(bar) { "context" } From 6a5e5b8f7a8d29d2688413cd3990df739b6b2de6 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 20 Dec 2022 21:40:47 -0700 Subject: [PATCH 41/67] Catch errors while evaluating node labels --- src/spectator/dsl/examples.cr | 6 +++++- src/spectator/dsl/groups.cr | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/spectator/dsl/examples.cr b/src/spectator/dsl/examples.cr index 4524001..8a08116 100644 --- a/src/spectator/dsl/examples.cr +++ b/src/spectator/dsl/examples.cr @@ -125,7 +125,11 @@ module Spectator::DSL {% if what.is_a?(StringLiteral) || what.is_a?(NilLiteral) %} {{what}} {% elsif what.is_a?(StringInterpolation) %} - {{@type.name}}.new.eval { {{what}} } + {{@type.name}}.new.eval do + {{what}} + rescue e + "" + end {% else %} {{what.stringify}} {% end %} diff --git a/src/spectator/dsl/groups.cr b/src/spectator/dsl/groups.cr index f595791..da06906 100644 --- a/src/spectator/dsl/groups.cr +++ b/src/spectator/dsl/groups.cr @@ -137,7 +137,11 @@ module Spectator::DSL what.is_a?(NilLiteral) %} {{what}} {% elsif what.is_a?(StringInterpolation) %} - {{@type.name}}.new.eval { {{what}} } + {{@type.name}}.new.eval do + {{what}} + rescue e + "" + end {% else %} {{what.stringify}} {% end %} From fd372226ab7db998d321368d7c00a5dc4dc072c3 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Wed, 21 Dec 2022 18:51:09 -0700 Subject: [PATCH 42/67] Revert "Use separate context for example name interpolation" This reverts commit d46698d81a444d192633bd599beb2e8bf07c53bb. --- CHANGELOG.md | 1 - src/spectator/dsl/examples.cr | 6 ++---- src/spectator/example.cr | 24 ++++++++++++++++++++++++ src/spectator/example_builder.cr | 11 ++++++++++- 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfbe8c2..fb7b645 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Expectations using 'should' syntax report file and line where the 'should' keyword is instead of the test start. -- String interpolation for example names/labels uses a separate context than the one used by the test. ## [0.11.5] - 2022-12-18 ### Added diff --git a/src/spectator/dsl/examples.cr b/src/spectator/dsl/examples.cr index 8a08116..703f2d3 100644 --- a/src/spectator/dsl/examples.cr +++ b/src/spectator/dsl/examples.cr @@ -125,10 +125,8 @@ module Spectator::DSL {% if what.is_a?(StringLiteral) || what.is_a?(NilLiteral) %} {{what}} {% elsif what.is_a?(StringInterpolation) %} - {{@type.name}}.new.eval do - {{what}} - rescue e - "" + ->(example : ::Spectator::Example) do + example.with_context(\{{@type.name}}) { {{what}} } end {% else %} {{what.stringify}} diff --git a/src/spectator/example.cr b/src/spectator/example.cr index 0c20ccb..04d69c9 100644 --- a/src/spectator/example.cr +++ b/src/spectator/example.cr @@ -28,6 +28,8 @@ module Spectator # Is pending if the example hasn't run. getter result : Result = PendingResult.new("Example not run") + @name_proc : Proc(Example, String)? + # Creates the example. # An instance to run the test code in is given by *context*. # The *entrypoint* defines the test code (typically inside *context*). @@ -45,6 +47,24 @@ module Spectator group << self if group end + # Creates the example. + # An instance to run the test code in is given by *context*. + # The *entrypoint* defines the test code (typically inside *context*). + # The *name* describes the purpose of the example. + # It can be a proc to be evaluated in the context of the example. + # The *location* tracks where the example exists in source code. + # The example will be assigned to *group* if it is provided. + # A set of *metadata* can be used for filtering and modifying example behavior. + # Note: The metadata will not be merged with the parent metadata. + def initialize(@context : Context, @entrypoint : self ->, + @name_proc : Example -> String, location : Location? = nil, + @group : ExampleGroup? = nil, metadata = nil) + super(nil, location, metadata) + + # Ensure group is linked. + group << self if group + end + # Creates a dynamic example. # A block provided to this method will be called as-if it were the test code for the example. # The block will be given this example instance as an argument. @@ -98,6 +118,10 @@ module Spectator begin @result = Harness.run do + if proc = @name_proc + self.name = proc.call(self) + end + @group.try(&.call_before_all) if (parent = @group) parent.call_around_each(procsy).call diff --git a/src/spectator/example_builder.cr b/src/spectator/example_builder.cr index ae5866a..23398d2 100644 --- a/src/spectator/example_builder.cr +++ b/src/spectator/example_builder.cr @@ -8,7 +8,7 @@ module Spectator # Constructs examples. # Call `#build` to produce an `Example`. class ExampleBuilder < NodeBuilder - @name : String? + @name : Proc(Example, String) | String? # Creates the builder. # A proc provided by *context_builder* is used to create a unique `Context` for each example produced by `#build`. @@ -18,6 +18,15 @@ module Spectator @name : String? = nil, @location : Location? = nil, @metadata : Metadata? = nil) end + # Creates the builder. + # A proc provided by *context_builder* is used to create a unique `Context` for each example produced by `#build`. + # The *entrypoint* indicates the proc used to invoke the test code in the example. + # The *name* is an interpolated string that runs in the context of the example. + # *location*, and *metadata* will be applied to the `Example` produced by `#build`. + def initialize(@context_builder : -> Context, @entrypoint : Example ->, + @name : Example -> String, @location : Location? = nil, @metadata : Metadata? = nil) + end + # Constructs an example with previously defined attributes and context. # The *parent* is an already constructed example group to nest the new example under. # It can be nil if the new example won't have a parent. From abbd6ffd71973a7ce7bdea24218a9fb01974a9d2 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Mon, 23 Jan 2023 11:55:52 -0700 Subject: [PATCH 43/67] Fix splat argument expansion in method redefinition The constructed previous_def call was malformed for stub methods. Resolves the original issue in https://github.com/icy-arctic-fox/spectator/issues/49 --- spec/issues/github_issue_49_spec.cr | 6 ++++++ src/spectator/mocks/stubbable.cr | 8 +++----- 2 files changed, 9 insertions(+), 5 deletions(-) create mode 100644 spec/issues/github_issue_49_spec.cr diff --git a/spec/issues/github_issue_49_spec.cr b/spec/issues/github_issue_49_spec.cr new file mode 100644 index 0000000..d6a3417 --- /dev/null +++ b/spec/issues/github_issue_49_spec.cr @@ -0,0 +1,6 @@ +require "../spec_helper" + +# https://github.com/icy-arctic-fox/spectator/issues/49 +Spectator.describe "GitHub Issue #49" do + mock File +end diff --git a/src/spectator/mocks/stubbable.cr b/src/spectator/mocks/stubbable.cr index 119111d..3a14887 100644 --- a/src/spectator/mocks/stubbable.cr +++ b/src/spectator/mocks/stubbable.cr @@ -133,13 +133,12 @@ module Spectator if method.splat_index method.args.each_with_index do |arg, i| if i == method.splat_index - original += '*' if arg.internal_name && arg.internal_name.size > 0 - original += "#{arg.internal_name}, " + original += "*#{arg.internal_name}, " end original += "**#{method.double_splat}, " if method.double_splat elsif i > method.splat_index - original += "#{arg.name}: #{arg.internal_name}" + original += "#{arg.name}: #{arg.internal_name}, " else original += "#{arg.internal_name}, " end @@ -283,9 +282,8 @@ module Spectator if method.splat_index method.args.each_with_index do |arg, i| if i == method.splat_index - original += '*' if arg.internal_name && arg.internal_name.size > 0 - original += "#{arg.internal_name}, " + original += "*#{arg.internal_name}, " end original += "**#{method.double_splat}, " if method.double_splat elsif i > method.splat_index From a5e8f11e1188f2ec8acfbd7d1fba5a83833828ec Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Mon, 23 Jan 2023 16:02:30 -0700 Subject: [PATCH 44/67] Store type to reduce a bit of bloat --- src/spectator/mocks/stubbable.cr | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/spectator/mocks/stubbable.cr b/src/spectator/mocks/stubbable.cr index 3a14887..3385ad4 100644 --- a/src/spectator/mocks/stubbable.cr +++ b/src/spectator/mocks/stubbable.cr @@ -537,6 +537,7 @@ module Spectator # Get the value as-is from the stub. # This will be compiled as a union of all known stubbed value types. %value = {{stub}}.call({{call}}) + %type = {{type}} # Attempt to cast the value to the method's return type. # If successful, which it will be in most cases, return it. @@ -547,12 +548,12 @@ module Spectator %cast {% elsif fail_cast == :raise %} # Check if nil was returned by the stub and if its okay to return it. - if %value.nil? && {{type}}.nilable? + if %value.nil? && %type.nilable? # Value was nil and nil is allowed to be returned. - %cast.as({{type}}) + %type.cast(%cast) elsif %cast.nil? # The stubbed value was something else entirely and cannot be cast to the return type. - raise TypeCastError.new("#{_spectator_stubbed_name} received message #{ {{call}} } and is attempting to return a `#{%value.class}`, but returned type must be `#{ {{type}} }`.") + raise TypeCastError.new("#{_spectator_stubbed_name} received message #{ {{call}} } and is attempting to return a `#{%value.class}`, but returned type must be `#{%type}`.") else # Types match and value can be returned as cast type. %cast From cb89589155cedc89dd9faeeca864e97266284edd Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Wed, 25 Jan 2023 16:09:16 -0700 Subject: [PATCH 45/67] Compiler bug when using unsafe_as --- src/spectator/mocks/stubbable.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spectator/mocks/stubbable.cr b/src/spectator/mocks/stubbable.cr index 3385ad4..6c5f7ad 100644 --- a/src/spectator/mocks/stubbable.cr +++ b/src/spectator/mocks/stubbable.cr @@ -550,7 +550,7 @@ module Spectator # Check if nil was returned by the stub and if its okay to return it. if %value.nil? && %type.nilable? # Value was nil and nil is allowed to be returned. - %type.cast(%cast) + %cast.unsafe_as({{type}}) elsif %cast.nil? # The stubbed value was something else entirely and cannot be cast to the return type. raise TypeCastError.new("#{_spectator_stubbed_name} received message #{ {{call}} } and is attempting to return a `#{%value.class}`, but returned type must be `#{%type}`.") From 7149ef7df572eddd3037e0e2a0ee922a66332ec5 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Thu, 26 Jan 2023 16:12:54 -0700 Subject: [PATCH 46/67] Revert "Compiler bug when using unsafe_as" This reverts commit cb89589155cedc89dd9faeeca864e97266284edd. --- src/spectator/mocks/stubbable.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spectator/mocks/stubbable.cr b/src/spectator/mocks/stubbable.cr index 6c5f7ad..3385ad4 100644 --- a/src/spectator/mocks/stubbable.cr +++ b/src/spectator/mocks/stubbable.cr @@ -550,7 +550,7 @@ module Spectator # Check if nil was returned by the stub and if its okay to return it. if %value.nil? && %type.nilable? # Value was nil and nil is allowed to be returned. - %cast.unsafe_as({{type}}) + %type.cast(%cast) elsif %cast.nil? # The stubbed value was something else entirely and cannot be cast to the return type. raise TypeCastError.new("#{_spectator_stubbed_name} received message #{ {{call}} } and is attempting to return a `#{%value.class}`, but returned type must be `#{%type}`.") From 528ad7257da3f01c164df619f210a8336a556ad7 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Thu, 26 Jan 2023 16:17:29 -0700 Subject: [PATCH 47/67] Disable GitHub issue 49 spec for now --- spec/issues/github_issue_49_spec.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/issues/github_issue_49_spec.cr b/spec/issues/github_issue_49_spec.cr index d6a3417..6161a57 100644 --- a/spec/issues/github_issue_49_spec.cr +++ b/spec/issues/github_issue_49_spec.cr @@ -2,5 +2,5 @@ require "../spec_helper" # https://github.com/icy-arctic-fox/spectator/issues/49 Spectator.describe "GitHub Issue #49" do - mock File + # mock File end From 24a860ea1161eff12d81cc7d5cf0c24044ac4bfe Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Thu, 26 Jan 2023 16:18:26 -0700 Subject: [PATCH 48/67] Add reference to new issue https://github.com/icy-arctic-fox/spectator/issues/51 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb7b645..fe4331d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added support for string interpolation in context names/labels. ### Fixed -- Fix invalid syntax (unterminated call) when recording calls to stubs with an un-named splat. +- Fix invalid syntax (unterminated call) when recording calls to stubs with an un-named splat. [#51](https://github.com/icy-arctic-fox/spectator/issues/51) ### Changed - Expectations using 'should' syntax report file and line where the 'should' keyword is instead of the test start. From 9ea5c261b15f4bb526aa5e1e3d1f93125f2e3168 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Thu, 26 Jan 2023 16:19:55 -0700 Subject: [PATCH 49/67] Add entry for GitHub issue 49 https://github.com/icy-arctic-fox/spectator/issues/49 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe4331d..8d93c8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix invalid syntax (unterminated call) when recording calls to stubs with an un-named splat. [#51](https://github.com/icy-arctic-fox/spectator/issues/51) +- Fix malformed method signature when using named splat with keyword arguments in mocked type. [#49](https://github.com/icy-arctic-fox/spectator/issues/49) ### Changed - Expectations using 'should' syntax report file and line where the 'should' keyword is instead of the test start. From 735122a94ba9a86a7957df9e0a1b7b8cf697b32b Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Thu, 26 Jan 2023 16:21:33 -0700 Subject: [PATCH 50/67] Bump v0.11.6 --- CHANGELOG.md | 5 ++++- shard.yml | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d93c8c..ffd70d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] + +## [0.11.6] - 2023-01-26 ### Added - Added ability to cast types using the return value from expect/should statements with a type matcher. - Added support for string interpolation in context names/labels. @@ -445,7 +447,8 @@ This has been changed so that it compiles and raises an error at runtime with a First version ready for public use. -[Unreleased]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.5...master +[Unreleased]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.6...master +[0.11.6]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.5...v0.11.6 [0.11.5]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.4...v0.11.5 [0.11.4]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.3...v0.11.4 [0.11.3]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.2...v0.11.3 diff --git a/shard.yml b/shard.yml index a41bc90..06ba936 100644 --- a/shard.yml +++ b/shard.yml @@ -1,5 +1,5 @@ name: spectator -version: 0.11.5 +version: 0.11.6 description: | Feature-rich testing framework for Crystal inspired by RSpec. From 5c08427ca0440193fc304c3405f8f56c079a0773 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Thu, 26 Jan 2023 16:43:19 -0700 Subject: [PATCH 51/67] Add utility script to run nightly spec --- util/nightly.sh | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100755 util/nightly.sh diff --git a/util/nightly.sh b/util/nightly.sh new file mode 100755 index 0000000..460a839 --- /dev/null +++ b/util/nightly.sh @@ -0,0 +1,7 @@ +#!/usr/bin/env sh +set -e + +readonly image=crystallang/crystal:nightly +readonly code=/project + +docker run -it -v "$PWD:${code}" -w "${code}" "${image}" crystal spec "$@" From 726a2e1515f437027c20a61e90e423223a6961ae Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Thu, 26 Jan 2023 17:19:31 -0700 Subject: [PATCH 52/67] Add non-captured block argument Preparing for Crystal 1.8.0 https://github.com/crystal-lang/crystal/issues/8764 --- CHANGELOG.md | 1 + spec/spectator/dsl/mocks/double_spec.cr | 2 +- spec/spectator/dsl/mocks/mock_spec.cr | 44 ++++++++++---------- spec/spectator/dsl/mocks/null_double_spec.cr | 2 +- spec/spectator/mocks/double_spec.cr | 2 +- spec/spectator/mocks/mock_spec.cr | 4 +- spec/spectator/mocks/null_double_spec.cr | 2 +- src/spectator/context.cr | 2 +- src/spectator/dsl/expectations.cr | 2 +- src/spectator/error_result.cr | 2 +- src/spectator/example.cr | 6 +-- src/spectator/example_group.cr | 2 +- src/spectator/fail_result.cr | 2 +- src/spectator/formatting/components/block.cr | 4 +- src/spectator/harness.cr | 12 +++--- src/spectator/matchers/exception_matcher.cr | 2 +- src/spectator/pass_result.cr | 2 +- src/spectator/pending_result.cr | 2 +- 18 files changed, 48 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffd70d3..17b7220 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Expectations using 'should' syntax report file and line where the 'should' keyword is instead of the test start. +- Add non-captured block argument in preparation for Crystal 1.8.0. ## [0.11.5] - 2022-12-18 ### Added diff --git a/spec/spectator/dsl/mocks/double_spec.cr b/spec/spectator/dsl/mocks/double_spec.cr index 89f652c..5547ae0 100644 --- a/spec/spectator/dsl/mocks/double_spec.cr +++ b/spec/spectator/dsl/mocks/double_spec.cr @@ -168,7 +168,7 @@ Spectator.describe "Double DSL", :smoke do context "methods accepting blocks" do double(:test7) do - stub def foo + stub def foo(&) yield end diff --git a/spec/spectator/dsl/mocks/mock_spec.cr b/spec/spectator/dsl/mocks/mock_spec.cr index 05ef4c9..cd57cdc 100644 --- a/spec/spectator/dsl/mocks/mock_spec.cr +++ b/spec/spectator/dsl/mocks/mock_spec.cr @@ -40,17 +40,17 @@ Spectator.describe "Mock DSL", :smoke do arg end - def method4 : Symbol + def method4(&) : Symbol @_spectator_invocations << :method4 yield end - def method5 + def method5(&) @_spectator_invocations << :method5 yield.to_i end - def method6 + def method6(&) @_spectator_invocations << :method6 yield end @@ -60,7 +60,7 @@ Spectator.describe "Mock DSL", :smoke do {arg, args, kwarg, kwargs} end - def method8(arg, *args, kwarg, **kwargs) + def method8(arg, *args, kwarg, **kwargs, &) @_spectator_invocations << :method8 yield {arg, args, kwarg, kwargs} @@ -80,7 +80,7 @@ Spectator.describe "Mock DSL", :smoke do "stubbed" end - stub def method4 : Symbol + stub def method4(&) : Symbol yield :block end @@ -258,12 +258,12 @@ Spectator.describe "Mock DSL", :smoke do # NOTE: Abstract methods that yield must have yield functionality defined in the method. # This requires that yielding methods have a default implementation. # Just providing `&` in the arguments gets dropped by the compiler unless `yield` is in the method definition. - stub def method5 + stub def method5(&) yield end # NOTE: Another quirk where a default implementation must be provided because `&` is dropped. - stub def method6 : Symbol + stub def method6(&) : Symbol yield end @@ -381,12 +381,12 @@ Spectator.describe "Mock DSL", :smoke do # NOTE: Abstract methods that yield must have yield functionality defined in the method. # This requires that yielding methods have a default implementation. # Just providing `&` in the arguments gets dropped by the compiler unless `yield` is in the method definition. - stub def method5 + stub def method5(&) yield end # NOTE: Another quirk where a default implementation must be provided because `&` is dropped. - stub def method6 : Symbol + stub def method6(&) : Symbol yield end end @@ -454,12 +454,12 @@ Spectator.describe "Mock DSL", :smoke do # NOTE: Abstract methods that yield must have yield functionality defined in the method. # This requires that yielding methods have a default implementation. # Just providing `&` in the arguments gets dropped by the compiler unless `yield` is in the method definition. - stub def method5 + stub def method5(&) yield end # NOTE: Another quirk where a default implementation must be provided because `&` is dropped. - stub def method6 : Symbol + stub def method6(&) : Symbol yield end @@ -577,12 +577,12 @@ Spectator.describe "Mock DSL", :smoke do # NOTE: Abstract methods that yield must have yield functionality defined in the method. # This requires that yielding methods have a default implementation. # Just providing `&` in the arguments gets dropped by the compiler unless `yield` is in the method definition. - stub def method5 + stub def method5(&) yield end # NOTE: Another quirk where a default implementation must be provided because `&` is dropped. - stub def method6 : Symbol + stub def method6(&) : Symbol yield end end @@ -620,11 +620,11 @@ Spectator.describe "Mock DSL", :smoke do :original end - def method3 + def method3(&) yield end - def method4 : Int32 + def method4(&) : Int32 yield.to_i end @@ -749,11 +749,11 @@ Spectator.describe "Mock DSL", :smoke do :original end - def method3 + def method3(&) yield end - def method4 : Int32 + def method4(&) : Int32 yield.to_i end @@ -1108,17 +1108,17 @@ Spectator.describe "Mock DSL", :smoke do arg end - def method4 : Symbol + def method4(&) : Symbol @_spectator_invocations << :method4 yield end - def method5 + def method5(&) @_spectator_invocations << :method5 yield.to_i end - def method6 + def method6(&) @_spectator_invocations << :method6 yield end @@ -1128,7 +1128,7 @@ Spectator.describe "Mock DSL", :smoke do {arg, args, kwarg, kwargs} end - def method8(arg, *args, kwarg, **kwargs) + def method8(arg, *args, kwarg, **kwargs, &) @_spectator_invocations << :method8 yield {arg, args, kwarg, kwargs} @@ -1148,7 +1148,7 @@ Spectator.describe "Mock DSL", :smoke do "stubbed" end - stub def method4 : Symbol + stub def method4(&) : Symbol yield :block end diff --git a/spec/spectator/dsl/mocks/null_double_spec.cr b/spec/spectator/dsl/mocks/null_double_spec.cr index 1219f50..06d35ee 100644 --- a/spec/spectator/dsl/mocks/null_double_spec.cr +++ b/spec/spectator/dsl/mocks/null_double_spec.cr @@ -156,7 +156,7 @@ Spectator.describe "Null double DSL" do context "methods accepting blocks" do double(:test7) do - stub def foo + stub def foo(&) yield end diff --git a/spec/spectator/mocks/double_spec.cr b/spec/spectator/mocks/double_spec.cr index 41bfad2..e55c549 100644 --- a/spec/spectator/mocks/double_spec.cr +++ b/spec/spectator/mocks/double_spec.cr @@ -297,7 +297,7 @@ Spectator.describe Spectator::Double do arg end - stub def self.baz(arg) + stub def self.baz(arg, &) yield end end diff --git a/spec/spectator/mocks/mock_spec.cr b/spec/spectator/mocks/mock_spec.cr index 7d04fed..3ddd0fe 100644 --- a/spec/spectator/mocks/mock_spec.cr +++ b/spec/spectator/mocks/mock_spec.cr @@ -364,7 +364,7 @@ Spectator.describe Spectator::Mock do arg end - def self.baz(arg) + def self.baz(arg, &) yield end @@ -929,7 +929,7 @@ Spectator.describe Spectator::Mock do arg end - def self.baz(arg) + def self.baz(arg, &) yield end end diff --git a/spec/spectator/mocks/null_double_spec.cr b/spec/spectator/mocks/null_double_spec.cr index ad87ea9..a6fc7d2 100644 --- a/spec/spectator/mocks/null_double_spec.cr +++ b/spec/spectator/mocks/null_double_spec.cr @@ -259,7 +259,7 @@ Spectator.describe Spectator::NullDouble do arg end - stub def self.baz(arg) + stub def self.baz(arg, &) yield end end diff --git a/src/spectator/context.cr b/src/spectator/context.cr index b9f532a..15b9335 100644 --- a/src/spectator/context.cr +++ b/src/spectator/context.cr @@ -5,7 +5,7 @@ # The reason for this is to prevent name collision when using the DSL to define a spec. abstract class SpectatorContext # Evaluates the contents of a block within the scope of the context. - def eval + def eval(&) with self yield end diff --git a/src/spectator/dsl/expectations.cr b/src/spectator/dsl/expectations.cr index a35a15c..dba2e9b 100644 --- a/src/spectator/dsl/expectations.cr +++ b/src/spectator/dsl/expectations.cr @@ -182,7 +182,7 @@ module Spectator::DSL # expect(false).to be_true # end # ``` - def aggregate_failures(label = nil) + def aggregate_failures(label = nil, &) ::Spectator::Harness.current.aggregate_failures(label) do yield end diff --git a/src/spectator/error_result.cr b/src/spectator/error_result.cr index a4531fb..f58da20 100644 --- a/src/spectator/error_result.cr +++ b/src/spectator/error_result.cr @@ -11,7 +11,7 @@ module Spectator end # Calls the `error` method on *visitor*. - def accept(visitor) + def accept(visitor, &) visitor.error(yield self) end diff --git a/src/spectator/example.cr b/src/spectator/example.cr index 04d69c9..e18676c 100644 --- a/src/spectator/example.cr +++ b/src/spectator/example.cr @@ -164,7 +164,7 @@ module Spectator # The context casted to an instance of *klass* is provided as a block argument. # # TODO: Benchmark compiler performance using this method versus client-side casting in a proc. - protected def with_context(klass) + protected def with_context(klass, &) context = klass.cast(@context) with context yield end @@ -184,7 +184,7 @@ module Spectator end # Yields this example and all parent groups. - def ascend + def ascend(&) node = self while node yield node @@ -279,7 +279,7 @@ module Spectator # The block given to this method will be executed within the test context. # # TODO: Benchmark compiler performance using this method versus client-side casting in a proc. - protected def with_context(klass) + protected def with_context(klass, &) context = @example.cast_context(klass) with context yield end diff --git a/src/spectator/example_group.cr b/src/spectator/example_group.cr index 277be3c..55a3233 100644 --- a/src/spectator/example_group.cr +++ b/src/spectator/example_group.cr @@ -87,7 +87,7 @@ module Spectator delegate size, unsafe_fetch, to: @nodes # Yields this group and all parent groups. - def ascend + def ascend(&) group = self while group yield group diff --git a/src/spectator/fail_result.cr b/src/spectator/fail_result.cr index c283a80..082a0d2 100644 --- a/src/spectator/fail_result.cr +++ b/src/spectator/fail_result.cr @@ -24,7 +24,7 @@ module Spectator end # Calls the `failure` method on *visitor*. - def accept(visitor) + def accept(visitor, &) visitor.fail(yield self) end diff --git a/src/spectator/formatting/components/block.cr b/src/spectator/formatting/components/block.cr index 40cd5a8..22411a7 100644 --- a/src/spectator/formatting/components/block.cr +++ b/src/spectator/formatting/components/block.cr @@ -13,7 +13,7 @@ module Spectator::Formatting::Components end # Increases the indent by the a specific *amount* for the duration of the block. - private def indent(amount = INDENT) + private def indent(amount = INDENT, &) @indent += amount yield @indent -= amount @@ -23,7 +23,7 @@ module Spectator::Formatting::Components # The contents of the line should be generated by a block provided to this method. # Ensure that _only_ one line is produced by the block, # otherwise the indent will be lost. - private def line(io) + private def line(io, &) @indent.times { io << ' ' } yield io.puts diff --git a/src/spectator/harness.cr b/src/spectator/harness.cr index 6be48b9..1f9fa09 100644 --- a/src/spectator/harness.cr +++ b/src/spectator/harness.cr @@ -43,7 +43,7 @@ module Spectator # The value of `.current` is set to the harness for the duration of the test. # It will be reset after the test regardless of the outcome. # The result of running the test code will be returned. - def self.run : Result + def self.run(&) : Result with_harness do |harness| harness.run { yield } end @@ -53,7 +53,7 @@ module Spectator # The `.current` harness is set to the new harness for the duration of the block. # `.current` is reset to the previous value (probably nil) afterwards, even if the block raises. # The result of the block is returned. - private def self.with_harness + private def self.with_harness(&) previous = @@current begin @@current = harness = new @@ -70,7 +70,7 @@ module Spectator # Runs test code and produces a result based on the outcome. # The test code should be called from within the block given to this method. - def run : Result + def run(&) : Result elapsed, error = capture { yield } elapsed2, error2 = capture { run_deferred } run_cleanup @@ -106,7 +106,7 @@ module Spectator @cleanup << block end - def aggregate_failures(label = nil) + def aggregate_failures(label = nil, &) previous = @aggregate @aggregate = aggregate = [] of Expectation begin @@ -135,7 +135,7 @@ module Spectator # Yields to run the test code and returns information about the outcome. # Returns a tuple with the elapsed time and an error if one occurred (otherwise nil). - private def capture : Tuple(Time::Span, Exception?) + private def capture(&) : Tuple(Time::Span, Exception?) error = nil elapsed = Time.measure do error = catch { yield } @@ -146,7 +146,7 @@ module Spectator # Yields to run a block of code and captures exceptions. # If the block of code raises an error, the error is caught and returned. # If the block doesn't raise an error, then nil is returned. - private def catch : Exception? + private def catch(&) : Exception? yield rescue e e diff --git a/src/spectator/matchers/exception_matcher.cr b/src/spectator/matchers/exception_matcher.cr index adec663..b26d390 100644 --- a/src/spectator/matchers/exception_matcher.cr +++ b/src/spectator/matchers/exception_matcher.cr @@ -97,7 +97,7 @@ module Spectator::Matchers # Runs a block of code and returns the exception it threw. # If no exception was thrown, *nil* is returned. - private def capture_exception + private def capture_exception(&) exception = nil begin yield diff --git a/src/spectator/pass_result.cr b/src/spectator/pass_result.cr index 20e3b04..21ed6c5 100644 --- a/src/spectator/pass_result.cr +++ b/src/spectator/pass_result.cr @@ -9,7 +9,7 @@ module Spectator end # Calls the `pass` method on *visitor*. - def accept(visitor) + def accept(visitor, &) visitor.pass(yield self) end diff --git a/src/spectator/pending_result.cr b/src/spectator/pending_result.cr index cff38c5..57f7fd7 100644 --- a/src/spectator/pending_result.cr +++ b/src/spectator/pending_result.cr @@ -28,7 +28,7 @@ module Spectator end # Calls the `pending` method on the *visitor*. - def accept(visitor) + def accept(visitor, &) visitor.pending(yield self) end From 9cbb5d2cf74410b5ac2f408a75b46be5708ee4fc Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Mon, 27 Mar 2023 18:37:50 -0600 Subject: [PATCH 53/67] Workaround issue using Box with union Addresses issue found relating to https://gitlab.com/arctic-fox/spectator/-/issues/81 See https://github.com/crystal-lang/crystal/issues/11839 --- CHANGELOG.md | 2 ++ src/spectator/wrapper.cr | 29 ++++++++++++++++------------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17b7220..325b7c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed +- Fix memoized value (`let`) with a union type causing segfault. [#81](https://gitlab.com/arctic-fox/spectator/-/issues/81) ## [0.11.6] - 2023-01-26 ### Added diff --git a/src/spectator/wrapper.cr b/src/spectator/wrapper.cr index 9874dec..76f6f44 100644 --- a/src/spectator/wrapper.cr +++ b/src/spectator/wrapper.cr @@ -13,18 +13,13 @@ module Spectator # Creates a wrapper for the specified value. def initialize(value) - @pointer = Box.box(value) + @pointer = Value.new(value).as(Void*) end # Retrieves the previously wrapped value. # The *type* of the wrapped value must match otherwise an error will be raised. def get(type : T.class) : T forall T - {% begin %} - {% if T.nilable? %} - @pointer.null? ? nil : - {% end %} - Box(T).unbox(@pointer) - {% end %} + @pointer.unsafe_as(Value(T)).get end # Retrieves the previously wrapped value. @@ -39,12 +34,20 @@ module Spectator # type = wrapper.get { Int32 } # Returns Int32 # ``` def get(& : -> T) : T forall T - {% begin %} - {% if T.nilable? %} - @pointer.null? ? nil : - {% end %} - Box(T).unbox(@pointer) - {% end %} + @pointer.unsafe_as(Value(T)).get + end + + # Wrapper for a value. + # Similar to `Box`, but doesn't segfault on nil and unions. + private class Value(T) + # Creates the wrapper. + def initialize(@value : T) + end + + # Retrieves the value. + def get : T + @value + end end end end From 04f151fddf7ea1c6b2479eee8c8a22461bc0b9f0 Mon Sep 17 00:00:00 2001 From: Stuart Frost Date: Fri, 19 May 2023 19:39:22 +0100 Subject: [PATCH 54/67] Fix mocking example in README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 973b9a9..323b4ca 100644 --- a/README.md +++ b/README.md @@ -287,7 +287,7 @@ Spectator.describe Driver do # Call the mock method. subject.do_something(interface, dbl) # Verify everything went okay. - expect(interface).to have_received(:invoke).with(thing) + expect(interface).to have_received(:invoke).with(dbl) end end ``` From 4a630b1ebf1ea4324b3c4f0230376783ce5a92a1 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Mon, 16 Oct 2023 17:34:49 -0600 Subject: [PATCH 55/67] Bump version to v0.11.7 --- CHANGELOG.md | 5 +++-- shard.yml | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 325b7c5..4e23272 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [0.11.7] - 2023-10-16 ### Fixed - Fix memoized value (`let`) with a union type causing segfault. [#81](https://gitlab.com/arctic-fox/spectator/-/issues/81) @@ -450,7 +450,8 @@ This has been changed so that it compiles and raises an error at runtime with a First version ready for public use. -[Unreleased]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.6...master +[Unreleased]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.7...master +[0.11.7]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.6...v0.11.7 [0.11.6]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.5...v0.11.6 [0.11.5]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.4...v0.11.5 [0.11.4]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.3...v0.11.4 diff --git a/shard.yml b/shard.yml index 06ba936..78ab190 100644 --- a/shard.yml +++ b/shard.yml @@ -1,5 +1,5 @@ name: spectator -version: 0.11.6 +version: 0.11.7 description: | Feature-rich testing framework for Crystal inspired by RSpec. From 5520999b6d7619985a93935faf546667f54289ad Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 27 Jan 2024 11:16:57 -0700 Subject: [PATCH 56/67] Add spec for GitHub issue 55 https://github.com/icy-arctic-fox/spectator/issues/55 --- spec/issues/github_issue_55_spec.cr | 48 +++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 spec/issues/github_issue_55_spec.cr diff --git a/spec/issues/github_issue_55_spec.cr b/spec/issues/github_issue_55_spec.cr new file mode 100644 index 0000000..92c2b42 --- /dev/null +++ b/spec/issues/github_issue_55_spec.cr @@ -0,0 +1,48 @@ +require "../spec_helper" + +Spectator.describe "GitHub Issue #55" do + GROUP_NAME = "CallCenter" + + let(name) { "TimeTravel" } + let(source) { "my.time.travel.experiment" } + + class Analytics(T) + property start_time = Time.local + property end_time = Time.local + + def initialize(@brain_talker : T) + end + + def instrument(*, name, source, &) + @brain_talker.send(payload: { + :group => GROUP_NAME, + :name => name, + :source => source, + :start => start_time, + :end => end_time, + }, action: "analytics") + end + end + + double(:brain_talker, send: nil) + + let(brain_talker) { double(:brain_talker) } + let(analytics) { Analytics.new(brain_talker) } + + it "tracks the time it takes to run the block" do + analytics.start_time = expected_start_time = Time.local + expected_end_time = expected_start_time + 10.seconds + analytics.end_time = expected_end_time + 0.5.seconds # Offset to ensure non-exact match. + + analytics.instrument(name: name, source: source) do + end + + expect(brain_talker).to have_received(:send).with(payload: { + :group => GROUP_NAME, + :name => name, + :source => source, + :start => expected_start_time, + :end => be_within(1.second).of(expected_end_time), + }, action: "analytics") + end +end From b5fbc96195031cdcb2658dd0a324e4e73c0b6f9a Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 27 Jan 2024 11:17:19 -0700 Subject: [PATCH 57/67] Allow matchers to be used in case equality --- src/spectator/matchers/matcher.cr | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/spectator/matchers/matcher.cr b/src/spectator/matchers/matcher.cr index e54e55e..05adb81 100644 --- a/src/spectator/matchers/matcher.cr +++ b/src/spectator/matchers/matcher.cr @@ -1,3 +1,4 @@ +require "../value" require "./match_data" module Spectator::Matchers @@ -22,6 +23,19 @@ module Spectator::Matchers # A successful match with `#match` should normally fail for this method, and vice-versa. abstract def negated_match(actual : Expression(T)) : MatchData forall T + # Compares a matcher against a value. + # Enables composable matchers. + def ===(actual : Expression(T)) : Bool + match(actual).matched? + end + + # Compares a matcher against a value. + # Enables composable matchers. + def ===(other) : Bool + expression = Value.new(other) + match(expression).matched? + end + private def match_data_description(actual : Expression(T)) : String forall T match_data_description(actual.label) end From 556d4783bf3669cba6fab53db3ea42edba7981f4 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 27 Jan 2024 11:18:10 -0700 Subject: [PATCH 58/67] Support case equality of tuples, arrays, named tuples, and hashes in stub argument matching --- src/spectator/mocks/abstract_arguments.cr | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/spectator/mocks/abstract_arguments.cr b/src/spectator/mocks/abstract_arguments.cr index 442c1d2..fd9bac9 100644 --- a/src/spectator/mocks/abstract_arguments.cr +++ b/src/spectator/mocks/abstract_arguments.cr @@ -7,7 +7,7 @@ module Spectator end # Utility method for comparing two tuples considering special types. - private def compare_tuples(a : Tuple, b : Tuple) + private def compare_tuples(a : Tuple | Array, b : Tuple | Array) return false if a.size != b.size a.zip(b) do |a_value, b_value| @@ -18,14 +18,14 @@ module Spectator # Utility method for comparing two tuples considering special types. # Supports nilable tuples (ideal for splats). - private def compare_tuples(a : Tuple?, b : Tuple?) + private def compare_tuples(a : Tuple? | Array?, b : Tuple? | Array?) return false if a.nil? ^ b.nil? compare_tuples(a.not_nil!, b.not_nil!) end # Utility method for comparing two named tuples ignoring order. - private def compare_named_tuples(a : NamedTuple, b : NamedTuple) + private def compare_named_tuples(a : NamedTuple | Hash, b : NamedTuple | Hash) a.each do |k, v1| v2 = b.fetch(k) { return false } return false unless compare_values(v1, v2) @@ -50,6 +50,18 @@ module Spectator else a == b end + when Tuple, Array + if b.is_a?(Tuple) || b.is_a?(Array) + compare_tuples(a, b) + else + a === b + end + when NamedTuple, Hash + if b.is_a?(NamedTuple) || b.is_a?(Hash) + compare_named_tuples(a, b) + else + a === b + end else a === b end From 526a998e4183e3d24621d7413d44eae91003e8be Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 27 Jan 2024 11:25:25 -0700 Subject: [PATCH 59/67] Shorten compare_values case statements --- src/spectator/mocks/abstract_arguments.cr | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/spectator/mocks/abstract_arguments.cr b/src/spectator/mocks/abstract_arguments.cr index fd9bac9..4a6f75f 100644 --- a/src/spectator/mocks/abstract_arguments.cr +++ b/src/spectator/mocks/abstract_arguments.cr @@ -45,23 +45,14 @@ module Spectator when Range # Ranges can only be matched against if their right side is comparable. # Ensure the right side is comparable, otherwise compare directly. - if b.is_a?(Comparable(typeof(b))) - a === b - else - a == b - end + return a === b if b.is_a?(Comparable(typeof(b))) + a == b when Tuple, Array - if b.is_a?(Tuple) || b.is_a?(Array) - compare_tuples(a, b) - else - a === b - end + return compare_tuples(a, b) if b.is_a?(Tuple) || b.is_a?(Array) + a === b when NamedTuple, Hash - if b.is_a?(NamedTuple) || b.is_a?(Hash) - compare_named_tuples(a, b) - else - a === b - end + return compare_named_tuples(a, b) if b.is_a?(NamedTuple) || b.is_a?(Hash) + a === b else a === b end From edb20e5b2f2e57c4326bad7a0fca374342adc8c5 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 27 Jan 2024 11:25:59 -0700 Subject: [PATCH 60/67] Additional handling when comparing ranges against unexpected types --- src/spectator/matchers/range_matcher.cr | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/spectator/matchers/range_matcher.cr b/src/spectator/matchers/range_matcher.cr index 8c31810..8a9a307 100644 --- a/src/spectator/matchers/range_matcher.cr +++ b/src/spectator/matchers/range_matcher.cr @@ -29,7 +29,26 @@ module Spectator::Matchers # Checks whether the matcher is satisfied with the expression given to it. private def match?(actual : Expression(T)) : Bool forall T - expected.value.includes?(actual.value) + actual_value = actual.value + expected_value = expected.value + if expected_value.is_a?(Range) && actual_value.is_a?(Comparable) + return match_impl?(expected_value, actual_value) + end + return false unless actual_value.is_a?(Comparable(typeof(expected_value.begin))) + expected_value.includes?(actual_value) + end + + private def match_impl?(expected_value : Range(B, E), actual_value : Comparable(B)) : Bool forall B, E + expected_value.includes?(actual_value) + end + + private def match_impl?(expected_value : Range(B, E), actual_value : T) : Bool forall B, E, T + return false unless actual_value.is_a?(B) || actual_value.is_a?(Comparable(B)) + expected_value.includes?(actual_value) + end + + private def match_impl?(expected_value : Range(Number, Number), actual_value : Number) : Bool + expected_value.includes?(actual_value) end # Message displayed when the matcher isn't satisfied. From 9b1d400ee1f0edf0b979b0d113ea618b16e0deed Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 27 Jan 2024 11:29:11 -0700 Subject: [PATCH 61/67] Update CHANGELOG --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e23272..7f7943a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] +### Added +- Added ability to use matchers for case equality. [#55](https://github.com/icy-arctic-fox/spectator/issues/55) +- Added support for nested case equality when checking arguments with Array, Tuple, Hash, and NamedTuple. + +### Fixed +- Fixed some issues with the `be_within` matcher when used with expected and union types. + ## [0.11.7] - 2023-10-16 ### Fixed - Fix memoized value (`let`) with a union type causing segfault. [#81](https://gitlab.com/arctic-fox/spectator/-/issues/81) From f39ceb8eba897cf634fee1922543e8a7e7125f02 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 3 Feb 2024 07:56:57 -0700 Subject: [PATCH 62/67] Release v0.12.0 --- CHANGELOG.md | 5 +++-- shard.yml | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f7943a..278c53c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [0.12.0] - 2024-02-03 ### Added - Added ability to use matchers for case equality. [#55](https://github.com/icy-arctic-fox/spectator/issues/55) - Added support for nested case equality when checking arguments with Array, Tuple, Hash, and NamedTuple. @@ -458,7 +458,8 @@ This has been changed so that it compiles and raises an error at runtime with a First version ready for public use. -[Unreleased]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.7...master +[Unreleased]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.12.0...master +[0.12.0]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.7...v0.12.0 [0.11.7]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.6...v0.11.7 [0.11.6]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.5...v0.11.6 [0.11.5]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.4...v0.11.5 diff --git a/shard.yml b/shard.yml index 78ab190..559754f 100644 --- a/shard.yml +++ b/shard.yml @@ -1,5 +1,5 @@ name: spectator -version: 0.11.7 +version: 0.12.0 description: | Feature-rich testing framework for Crystal inspired by RSpec. From 287758e6af784200a4a51a60f364211fbd1ff6c4 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 3 Feb 2024 08:05:02 -0700 Subject: [PATCH 63/67] Update README to point at v0.12.0 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 323b4ca..00b5eb9 100644 --- a/README.md +++ b/README.md @@ -25,7 +25,7 @@ Add this to your application's `shard.yml`: development_dependencies: spectator: gitlab: arctic-fox/spectator - version: ~> 0.11.0 + version: ~> 0.12.0 ``` Usage From 73e16862191d821951d72120110c7bdca1062b6a Mon Sep 17 00:00:00 2001 From: Grant Birkinbine Date: Sun, 11 Aug 2024 15:28:19 -0700 Subject: [PATCH 64/67] fix: `Error: expecting token 'CONST', not '::'` --- src/spectator/dsl/mocks.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spectator/dsl/mocks.cr b/src/spectator/dsl/mocks.cr index ab05636..18fe1fa 100644 --- a/src/spectator/dsl/mocks.cr +++ b/src/spectator/dsl/mocks.cr @@ -431,7 +431,7 @@ module Spectator::DSL # This isn't required, but new_mock() should still find this type. ::Spectator::DSL::Mocks::TYPES << {type.id.symbolize, @type.name(generic_args: false).symbolize, resolved.name.symbolize} %} - ::Spectator::Mock.inject({{base}}, ::{{resolved.name}}, {{**value_methods}}) {{block}} + ::Spectator::Mock.inject({{base}}, {{resolved.name}}, {{**value_methods}}) {{block}} end # Targets a stubbable object (such as a mock or double) for operations. From a634046a86c689835de09f4fb96a488d6657c25a Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 13 Aug 2024 18:42:22 -0600 Subject: [PATCH 65/67] Conditionally insert top-level namespace (double colon) --- src/spectator/dsl/mocks.cr | 4 ++-- src/spectator/mocks/mock.cr | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/spectator/dsl/mocks.cr b/src/spectator/dsl/mocks.cr index 18fe1fa..19645fc 100644 --- a/src/spectator/dsl/mocks.cr +++ b/src/spectator/dsl/mocks.cr @@ -226,7 +226,7 @@ module Spectator::DSL # Store information about how the mock is defined and its context. # This is important for constructing an instance of the mock later. - ::Spectator::DSL::Mocks::TYPES << {type.id.symbolize, @type.name(generic_args: false).symbolize, "::#{resolved.name}::#{mock_type_name}".id.symbolize} + ::Spectator::DSL::Mocks::TYPES << {type.id.symbolize, @type.name(generic_args: false).symbolize, "#{"::".id unless resolved.name.starts_with?("::")}#{resolved.name}::#{mock_type_name}".id.symbolize} base = if resolved.class? :class @@ -237,7 +237,7 @@ module Spectator::DSL end %} {% begin %} - {{base.id}} ::{{resolved.name}} + {{base.id}} {{"::".id unless resolved.name.starts_with?("::")}}{{resolved.name}} ::Spectator::Mock.define_subtype({{base}}, {{type.id}}, {{mock_type_name}}, {{name}}, {{**value_methods}}) {{block}} end {% end %} diff --git a/src/spectator/mocks/mock.cr b/src/spectator/mocks/mock.cr index 87e8e23..d2a1fde 100644 --- a/src/spectator/mocks/mock.cr +++ b/src/spectator/mocks/mock.cr @@ -149,7 +149,7 @@ module Spectator macro inject(base, type_name, name = nil, **value_methods, &block) {% begin %} {% if name %}@[::Spectator::StubbedName({{name}})]{% end %} - {{base.id}} ::{{type_name.id}} + {{base.id}} {{"::".id unless type_name.id.starts_with?("::")}}{{type_name.id}} include ::Spectator::Mocked extend ::Spectator::StubbedType From a93908d507977db1314f71a709a88444bf49191b Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 13 Aug 2024 18:51:00 -0600 Subject: [PATCH 66/67] Fix usage of deprecated double splat syntax in macros --- src/spectator/dsl/mocks.cr | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/spectator/dsl/mocks.cr b/src/spectator/dsl/mocks.cr index 19645fc..5eff1a9 100644 --- a/src/spectator/dsl/mocks.cr +++ b/src/spectator/dsl/mocks.cr @@ -31,7 +31,7 @@ module Spectator::DSL ::Spectator::DSL::Mocks::TYPES << {name.id.symbolize, @type.name(generic_args: false).symbolize, double_type_name.symbolize} %} # Define the plain double type. - ::Spectator::Double.define({{double_type_name}}, {{name}}, {{**value_methods}}) do + ::Spectator::Double.define({{double_type_name}}, {{name}}, {{value_methods.double_splat}}) do # Returns a new double that responds to undefined methods with itself. # See: `NullDouble` def as_null_object @@ -43,7 +43,7 @@ module Spectator::DSL {% begin %} # Define a matching null double type. - ::Spectator::NullDouble.define({{null_double_type_name}}, {{name}}, {{**value_methods}}) {{block}} + ::Spectator::NullDouble.define({{null_double_type_name}}, {{name}}, {{value_methods.double_splat}}) {{block}} {% end %} end @@ -94,9 +94,9 @@ module Spectator::DSL begin %double = {% if found_tuple %} - {{found_tuple[2].id}}.new({{**value_methods}}) + {{found_tuple[2].id}}.new({{value_methods.double_splat}}) {% else %} - ::Spectator::LazyDouble.new({{name}}, {{**value_methods}}) + ::Spectator::LazyDouble.new({{name}}, {{value_methods.double_splat}}) {% end %} ::Spectator::Harness.current?.try(&.cleanup { %double._spectator_reset }) %double @@ -176,7 +176,7 @@ module Spectator::DSL # See `#def_double`. macro double(name, **value_methods, &block) {% begin %} - {% if @def %}new_double{% else %}def_double{% end %}({{name}}, {{**value_methods}}) {{block}} + {% if @def %}new_double{% else %}def_double{% end %}({{name}}, {{value_methods.double_splat}}) {{block}} {% end %} end @@ -189,7 +189,7 @@ module Spectator::DSL # expect(dbl.foo).to eq(42) # ``` macro double(**value_methods) - ::Spectator::LazyDouble.new({{**value_methods}}) + ::Spectator::LazyDouble.new({{value_methods.double_splat}}) end # Defines a new mock type. @@ -238,7 +238,7 @@ module Spectator::DSL {% begin %} {{base.id}} {{"::".id unless resolved.name.starts_with?("::")}}{{resolved.name}} - ::Spectator::Mock.define_subtype({{base}}, {{type.id}}, {{mock_type_name}}, {{name}}, {{**value_methods}}) {{block}} + ::Spectator::Mock.define_subtype({{base}}, {{type.id}}, {{mock_type_name}}, {{name}}, {{value_methods.double_splat}}) {{block}} end {% end %} end @@ -321,7 +321,7 @@ module Spectator::DSL macro mock(type, **value_methods, &block) {% raise "First argument of `mock` must be a type name, not #{type}" unless type.is_a?(Path) || type.is_a?(Generic) || type.is_a?(Union) || type.is_a?(Metaclass) || type.is_a?(TypeNode) %} {% begin %} - {% if @def %}new_mock{% else %}def_mock{% end %}({{type}}, {{**value_methods}}) {{block}} + {% if @def %}new_mock{% else %}def_mock{% end %}({{type}}, {{value_methods.double_splat}}) {{block}} {% end %} end @@ -431,7 +431,7 @@ module Spectator::DSL # This isn't required, but new_mock() should still find this type. ::Spectator::DSL::Mocks::TYPES << {type.id.symbolize, @type.name(generic_args: false).symbolize, resolved.name.symbolize} %} - ::Spectator::Mock.inject({{base}}, {{resolved.name}}, {{**value_methods}}) {{block}} + ::Spectator::Mock.inject({{base}}, {{resolved.name}}, {{value_methods.double_splat}}) {{block}} end # Targets a stubbable object (such as a mock or double) for operations. From dcaa05531a3bba4855b8801b6a30bbd6c4e85cbc Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Tue, 13 Aug 2024 19:00:30 -0600 Subject: [PATCH 67/67] Release v0.12.1 --- CHANGELOG.md | 8 +++++++- shard.yml | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 278c53c..f2d8d66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.12.1] - 2024-08-13 +### Fixed +- Fixed some global namespace issues with Crystal 1.13. [#57](https://github.com/icy-arctic-fox/spectator/pull/57) Thanks @GrantBirki ! +- Remove usage of deprecated double splat in macros. + ## [0.12.0] - 2024-02-03 ### Added - Added ability to use matchers for case equality. [#55](https://github.com/icy-arctic-fox/spectator/issues/55) @@ -458,7 +463,8 @@ This has been changed so that it compiles and raises an error at runtime with a First version ready for public use. -[Unreleased]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.12.0...master +[Unreleased]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.12.1...master +[0.12.1]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.12.0...v0.12.1 [0.12.0]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.7...v0.12.0 [0.11.7]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.6...v0.11.7 [0.11.6]: https://gitlab.com/arctic-fox/spectator/-/compare/v0.11.5...v0.11.6 diff --git a/shard.yml b/shard.yml index 559754f..ac8b54f 100644 --- a/shard.yml +++ b/shard.yml @@ -1,5 +1,5 @@ name: spectator -version: 0.12.0 +version: 0.12.1 description: | Feature-rich testing framework for Crystal inspired by RSpec.