icy-arctic-fox / spectator

Feature-rich testing framework for Crystal inspired by RSpec.
https://gitlab.com/arctic-fox/spectator
MIT License
101 stars 4 forks source link

How to spec classes that contain class which shadow top-level primitives? #37

Closed postmodern closed 2 years ago

postmodern commented 2 years ago

An interesting problem. I'm attempting to port [this Ruby code]() which defines a Type class which contains other classes such as Int32. In Crystal I had to explicitly specify ::Int32 to avoid constant shadowing. However, Spectator is having issues with crystal_type_id. It appears that when describing Hexdump::Type the Int32 class contained within is shadowing the top level Int32.

Source

module Hexdump
  #
  # @api private
  #
  # @since 1.0.0
  #
  class Type

    enum Endian
      LITTLE
      BIG
      NETWORK = BIG

      def self.native : Endian
        int = 1_i32
        native_bytes = Bytes.new(pointerof(int).as(UInt8 *), sizeof(typeof(int)))
        big_endian_bytes = Bytes.new(sizeof(typeof(int)))

        IO::ByteFormat::BigEndian.encode(int,big_endian_bytes)

        if native_bytes == big_endian_bytes; BIG
        else                                 LITTLE
        end
      end
    end

    # The size in bytes of the type.
    #
    # @return [1, 2, 4, 8]
    getter size : ::Int32

    getter? signed : ::Bool

    # The endian-ness of the type.
    #
    # @return [:little, :big, nil]
    getter endian : Endian?

    #
    # Initializes the type.
    #
    # @param [Symbol] name
    #
    # @param [:little, :big, nil] endian
    #
    # @param [1, 2, 4, 8] size
    #
    # @param [Boolean] signed
    #
    # @raise [ArgumentError]
    #   Invalid `endian:` or `size:` values.
    #
    def initialize(@size : ::Int32, @signed : ::Bool, @endian : Endian? = nil)
    end

    #
    # Whether the type is unsigned.
    #
    # @return [Boolean]
    #
    def unsigned?
      !signed?
    end

    # The native endian-ness.
    NATIVE_ENDIAN = Endian.native

    #
    # Represents a signed integer type.
    #
    class Int < self

      #
      # Initializes the int type.
      #
      # @param [:little, :big] endian (NATIVE_ENDIAN)
      #   The endian-ness of the int type.
      #
      def initialize(size : Int32, endian : Endian? = NATIVE_ENDIAN)
        super(size: size, signed: true, endian: endian)
      end
    end

    class Int32 < Int

      #
      # @see Int#initialize
      #
      def initialize(endian : Endian? = NATIVE_ENDIAN)
        super(size: 4, endian: endian)
      end
    end

  end
end

Specs

require "./spec_helper"
require "../src/hexdump/type"

Spectator.describe Hexdump::Type do
  describe "#initialize" do
    let(size) { 4 }
    let(signed) { true }

    subject { described_class.new(size: size, signed: signed) }

    it "must set #size" do
      expect(subject.size).to eq(size)
    end

    it "must not set #endian" do
      expect(subject.endian).to be(nil)
    end

    it "must set #signed?" do
      expect(subject.signed?).to eq(signed)
    end

    context "when given endian:" do
      let(endian) { Hexdump::Type::Endian::BIG }

      subject do
        described_class.new(size: size, signed: signed, endian: endian)
      end

      it "must set #endian" do
        expect(subject.endian).to eq(endian)
      end
    end
  end

  describe "#signed?" do
    let(size) { 4 }

    subject { described_class.new(size: size, signed: signed) }

    context "when initialized with signed: true" do
      let(signed) { true }

      it do
        expect(subject.signed?).to be(true)
      end
    end

    context "when initialized with signed: false" do
      let(signed) { false }

      it do
        expect(subject.signed?).to be(false)
      end
    end
  end

  describe "#unsigned?" do
    let(size) { 4 }

    subject { described_class.new(size: size, signed: signed) }

    context "when initialized with signed: true" do
      let(signed) { true }

      it do
        expect(subject.unsigned?).to be(false)
      end
    end

    context "when initialized with signed: false" do
      let(signed) { false }

      it do
        expect(subject.unsigned?).to be(true)
      end
    end
  end

  describe Hexdump::Type::Int32 do
    it { expect(subject).is_a?(Hexdump::Type::Int) }

    describe "#initialize" do
      it "must default #signed? to true" do
        expect(subject.signed?).to be(true)
      end

      it "must default #size to 4" do
        expect(subject.size).to be(4)
      end

      it "must default #endian to NATIVE_ENDIAN" do
        expect(subject.endian).to eq(Hexdump::Type::NATIVE_ENDIAN)
      end
    end
  end
end

Error

$ crystal spec spec/type_spec.cr --error-trace
error in line 1
Error: while requiring "./spec/type_spec.cr"

In spec/type_spec.cr:4:11

 4 | Spectator.describe Hexdump::Type do
               ^-------
Error: expanding macro

In spec/type_spec.cr:4:1

 4 | Spectator.describe Hexdump::Type do
     ^
Error: expanding macro

There was a problem expanding macro 'describe'

Called macro defined in macro 'macro_140623426311824'

 46 | macro describe(description, *tags, **metadata, &block)

Which expanded to:

 >   1 |         class ::SpectatorTestContext
 >   2 |           describe(Hexdump::Type,  ) do
 >   3 |   describe("#initialize") do
 >   4 |     let(size) do
 >   5 |       4
 >   6 |     end
 >   7 |     let(signed) do
 >   8 |       true
 >   9 |     end
 >  10 |     subject do
 >  11 |       described_class.new(size: size, signed: signed)
 >  12 |     end
 >  13 |     it("must set #size") do
 >  14 |       (expect(subject.size)).to(eq(size))
 >  15 |     end
 >  16 |     it("must not set #endian") do
 >  17 |       (expect(subject.endian)).to(be(nil))
 >  18 |     end
 >  19 |     it("must set #signed?") do
 >  20 |       (expect(subject.signed?)).to(eq(signed))
 >  21 |     end
 >  22 |     context("when given endian:") do
 >  23 |       let(endian) do
 >  24 |         Hexdump::Type::Endian::BIG
 >  25 |       end
 >  26 |       subject do
 >  27 |         described_class.new(size: size, signed: signed, endian: endian)
 >  28 |       end
 >  29 |       it("must set #endian") do
 >  30 |         (expect(subject.endian)).to(eq(endian))
 >  31 |       end
 >  32 |     end
 >  33 |   end
 >  34 |   describe("#signed?") do
 >  35 |     let(size) do
 >  36 |       4
 >  37 |     end
 >  38 |     subject do
 >  39 |       described_class.new(size: size, signed: signed)
 >  40 |     end
 >  41 |     context("when initialized with signed: true") do
 >  42 |       let(signed) do
 >  43 |         true
 >  44 |       end
 >  45 |       it do
 >  46 |         (expect(subject.signed?)).to(be(true))
 >  47 |       end
 >  48 |     end
 >  49 |     context("when initialized with signed: false") do
 >  50 |       let(signed) do
 >  51 |         false
 >  52 |       end
 >  53 |       it do
 >  54 |         (expect(subject.signed?)).to(be(false))
 >  55 |       end
 >  56 |     end
 >  57 |   end
 >  58 |   describe("#unsigned?") do
 >  59 |     let(size) do
 >  60 |       4
 >  61 |     end
 >  62 |     subject do
 >  63 |       described_class.new(size: size, signed: signed)
 >  64 |     end
 >  65 |     context("when initialized with signed: true") do
 >  66 |       let(signed) do
 >  67 |         true
 >  68 |       end
 >  69 |       it do
 >  70 |         (expect(subject.unsigned?)).to(be(false))
 >  71 |       end
 >  72 |     end
 >  73 |     context("when initialized with signed: false") do
 >  74 |       let(signed) do
 >  75 |         false
 >  76 |       end
 >  77 |       it do
 >  78 |         (expect(subject.unsigned?)).to(be(true))
 >  79 |       end
 >  80 |     end
 >  81 |   end
 >  82 |   describe(Hexdump::Type::Int32) do
 >  83 |     it do
 >  84 |       expect(subject).is_a?(Hexdump::Type::Int)
 >  85 |     end
 >  86 |     describe("#initialize") do
 >  87 |       it("must default #signed? to true") do
 >  88 |         (expect(subject.signed?)).to(be(true))
 >  89 |       end
 >  90 |       it("must default #size to 4") do
 >  91 |         (expect(subject.size)).to(be(4))
 >  92 |       end
 >  93 |       it("must default #endian to NATIVE_ENDIAN") do
 >  94 |         (expect(subject.endian)).to(eq(Hexdump::Type::NATIVE_ENDIAN))
 >  95 |       end
 >  96 |     end
 >  97 |   end
 >  98 | end
 >  99 |         end
 > 100 |       
Error: expanding macro

In spec/type_spec.cr:15:11

 15 | it "must not set #endian" do
            ^-------
Error: expanding macro

In spec/type_spec.cr:14:1

 14 | 
      ^-
Error: expanding macro

In spec/type_spec.cr:14:1

 14 | 
      ^
Error: expanding macro

There was a problem expanding macro 'it'

Called macro defined in macro 'define_example'

 17 | macro it(what = nil, *tags, **metadata, &block)

Which expanded to:

 >  1 |         
 >  2 |         
 >  3 | 
 >  4 |         _spectator_metadata(__temp_36, :metadata,  )
 >  5 |         _spectator_metadata(__temp_65, __temp_36,  )
 >  6 | 
 >  7 |         
 >  8 |           
 >  9 | 
 > 10 |           private def __temp_66() : Nil
 > 11 |             (expect(subject.endian)).to(be(nil))
 > 12 |           end
 > 13 | 
 > 14 |           ::Spectator::DSL::Builder.add_example(
 > 15 |             _spectator_example_name("must not set #endian"),
 > 16 |             ::Spectator::Location.new("/data/home/postmodern/code/crystal/hexdump.cr/spec/type_spec.cr", 15, 17),
 > 17 |             -> { new.as(::Spectator::Context) },
 > 18 |             __temp_65
 > 19 |           ) do |example|
 > 20 |             example.with_context(SpectatorTestContext::Group__temp_51::Group__temp_55) do
 > 21 |               
 > 22 |                 __temp_66
 > 23 |               
 > 24 |             end
 > 25 |           end
 > 26 | 
 > 27 |         
 > 28 |       
Error: instantiating 'Spectator::Example#with_context(SpectatorTestContext::Group__temp_51::Group__temp_55.class)'

In spec/type_spec.cr:14:1

 14 | 
      ^
Error: expanding macro

There was a problem expanding macro 'it'

Called macro defined in macro 'define_example'

 17 | macro it(what = nil, *tags, **metadata, &block)

Which expanded to:

 >  1 |         
 >  2 |         
 >  3 | 
 >  4 |         _spectator_metadata(__temp_36, :metadata,  )
 >  5 |         _spectator_metadata(__temp_65, __temp_36,  )
 >  6 | 
 >  7 |         
 >  8 |           
 >  9 | 
 > 10 |           private def __temp_66() : Nil
 > 11 |             (expect(subject.endian)).to(be(nil))
 > 12 |           end
 > 13 | 
 > 14 |           ::Spectator::DSL::Builder.add_example(
 > 15 |             _spectator_example_name("must not set #endian"),
 > 16 |             ::Spectator::Location.new("/data/home/postmodern/code/crystal/hexdump.cr/spec/type_spec.cr", 15, 17),
 > 17 |             -> { new.as(::Spectator::Context) },
 > 18 |             __temp_65
 > 19 |           ) do |example|
 > 20 |             example.with_context(SpectatorTestContext::Group__temp_51::Group__temp_55) do
 > 21 |               
 > 22 |                 __temp_66
 > 23 |               
 > 24 |             end
 > 25 |           end
 > 26 | 
 > 27 |         
 > 28 |       
Error: instantiating 'Spectator::Example#with_context(SpectatorTestContext::Group__temp_51::Group__temp_55.class)'

In spec/type_spec.cr:14:1

 14 | 
      ^
Error: expanding macro

There was a problem expanding macro 'it'

Called macro defined in macro 'define_example'

 17 | macro it(what = nil, *tags, **metadata, &block)

Which expanded to:

 >  1 |         
 >  2 |         
 >  3 | 
 >  4 |         _spectator_metadata(__temp_36, :metadata,  )
 >  5 |         _spectator_metadata(__temp_65, __temp_36,  )
 >  6 | 
 >  7 |         
 >  8 |           
 >  9 | 
 > 10 |           private def __temp_66() : Nil
 > 11 |             (expect(subject.endian)).to(be(nil))
 > 12 |           end
 > 13 | 
 > 14 |           ::Spectator::DSL::Builder.add_example(
 > 15 |             _spectator_example_name("must not set #endian"),
 > 16 |             ::Spectator::Location.new("/data/home/postmodern/code/crystal/hexdump.cr/spec/type_spec.cr", 15, 17),
 > 17 |             -> { new.as(::Spectator::Context) },
 > 18 |             __temp_65
 > 19 |           ) do |example|
 > 20 |             example.with_context(SpectatorTestContext::Group__temp_51::Group__temp_55) do
 > 21 |               
 > 22 |                 __temp_66
 > 23 |               
 > 24 |             end
 > 25 |           end
 > 26 | 
 > 27 |         
 > 28 |       
Error: instantiating '__temp_66()'

In spec/type_spec.cr:16:31

 16 | expect(subject.endian).to be(nil)
                              ^-
Error: instantiating 'Spectator::Expectation::Target(Hexdump::Type::Endian | Nil)#to(Spectator::Matchers::ReferenceMatcher(Nil))'

In lib/spectator/src/spectator/expectation.cr:104:7

 104 | def to(matcher, message = nil) : Nil
       ^-
Error: instantiating 'to(Spectator::Matchers::ReferenceMatcher(Nil), Nil)'

In lib/spectator/src/spectator/expectation.cr:105:30

 105 | match_data = matcher.match(@expression)
                            ^----
Error: instantiating 'Spectator::Matchers::ReferenceMatcher(Nil)#match(Spectator::Expression(Hexdump::Type::Endian | Nil)+)'

In lib/spectator/src/spectator/matchers/standard_matcher.cr:27:10

 27 | if match?(actual)
         ^-----
Error: instantiating 'match?(Spectator::Expression(Hexdump::Type::Endian | Nil)+)'

In lib/spectator/src/spectator/matchers/reference_matcher.cr:21:28

 21 | actual.value.class == value.class && actual.value == value
                         ^
Error: instantiating '(Hexdump::Type::Endian.class | Nil.class)#==(Nil.class)'

In /var/lib/snapd/snap/crystal/793/share/crystal/src/class.cr:18:5

 18 | crystal_type_id == other.crystal_type_id
      ^--------------
Error: instantiating 'crystal_type_id()'

In /var/lib/snapd/snap/crystal/793/share/crystal/src/primitives.cr:32:25

 32 | def crystal_type_id : Int32
                            ^----
Error: method Hexdump::Type::Endian.crystal_type_id must return Int32 but it is returning Hexdump::Type::Int32

Hexdump::Type::Int32 trace:

  /var/lib/snapd/snap/crystal/793/share/crystal/src/primitives.cr:32

      def crystal_type_id : Int32
icy-arctic-fox commented 2 years ago

I've found two issues:

  1. Hexdump::Type::Int#initialize has a size parameter. That uses Int32 instead of ::Int32, so it accidentally references the Int32 class below it.
  2. The use of .to be(nil) doesn't behave correctly against value-types. In this instance, Endian is nillable. The be matcher uses the #same method for comparison. But this is only found on Nil and reference types. Spectator has custom logic for values, but didn't handle nil and value types quite right. This has been fixed and pushed to master.

After addressing both of these issues, the spec passes.

Alternatively, for 2, be_nil can be used in place of be(nil).

postmodern commented 2 years ago

Could a special case be added for be(nil) and Value types?

icy-arctic-fox commented 2 years ago

Could a special case be added for be(nil) and Value types?

Yup, one has been added and is on master.

postmodern commented 2 years ago

I've made the suggested changes and updated spectator, but now I'm hitting this issue when using .to be(...) to compare entries in a Hash.

  TYPES = Hash(Symbol,Type).new
  TYPES[:uint8] = Type::UInt8.new
  TYPES[:byte] = TYPES[:uint8]
require "./spec_helper"
require "../src/hexdump/types"

Spectator.describe "Hexdump::TYPES" do
  subject { Hexdump::TYPES }

  describe "byte" do
    it "must be an alias to uint8" do
      expect(subject[:byte]).to be(subject[:uint8])
    end
  end
In spec/types_spec.cr:9:31

 9 | expect(subject[:byte]).to be(subject[:uint8])
                             ^-
Error: instantiating 'Spectator::Expectation::Target(Hexdump::Type)#to(Spectator::Matchers::ReferenceMatcher(Hexdump::Type))'

In lib/spectator/src/spectator/expectation.cr:104:7

 104 | def to(matcher, message = nil) : Nil
       ^-
Error: instantiating 'to(Spectator::Matchers::ReferenceMatcher(Hexdump::Type), Nil)'

In lib/spectator/src/spectator/expectation.cr:105:30

 105 | match_data = matcher.match(@expression)
                            ^----
Error: instantiating 'Spectator::Matchers::ReferenceMatcher(Hexdump::Type)#match(Spectator::Expression(Hexdump::Type)+)'

In lib/spectator/src/spectator/matchers/standard_matcher.cr:27:10

 27 | if match?(actual)
         ^-----
Error: instantiating 'match?(Spectator::Expression(Hexdump::Type)+)'

In lib/spectator/src/spectator/matchers/reference_matcher.cr:20:15

 20 | value.same?(actual.value)
            ^----
Error: instantiating 'Hexdump::Type+#same?(Hexdump::Type+)'

In /var/lib/snapd/snap/crystal/793/share/crystal/src/reference.cr:29:5

 29 | object_id == other.object_id
      ^--------
Error: instantiating 'object_id()'

In /var/lib/snapd/snap/crystal/793/share/crystal/src/primitives.cr:50:19

 50 | def object_id : UInt64
                      ^-----
Error: method Hexdump::Type+#object_id must return UInt64 but it is returning Hexdump::Type::UInt64

Hexdump::Type::UInt64 trace:

  /var/lib/snapd/snap/crystal/793/share/crystal/src/primitives.cr:50

      def object_id : UInt64