Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not match expectations outside specs #13079

Merged

Conversation

HertzDevil
Copy link
Contributor

Some calls of #should are done outside spec examples (more specifically, an it or before_each), which means if the expectation does not match, the Spec::AssertionFailed exception is not handled and will stop any subsequent specs from being added and run. This PR fixes those occurrences.

They are detected by monkeypatching Spec:

module Spec
  class_property? running_spec = false

  module ObjectExtensions
    def should(expectation, ...)
      STDERR.puts "expectation outside spec! #{file}:#{line}" unless Spec.running_spec?
      # ...
    end

    # ditto for the other 4 overloads of `#should` and `#should_not`
  end
end

Spec.before_each { Spec.running_spec = true }
Spec.after_each { Spec.running_spec = false }

@HertzDevil HertzDevil added kind:bug A bug in the code. Does not apply to documentation, specs, etc. kind:specs labels Feb 15, 2023
@oprypin
Copy link
Member

oprypin commented Feb 15, 2023

They are detected by monkeypatching Spec:

There's probably some value in making this permanent, maybe with a flag.
But that's just a side comment


Signal::CHLD.trap do
child_process = chan.receive
Process.exists?(child_process.pid).should be_false
existed.send(Process.exists?(child_process.pid))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An assertion failure here is an unhandled exception for a signal handler, which would have been a fatal runtime error, so this spec requires some non-trivial editing.

{% end %}

raise "BUG: failed to compile dynamic library" unless $?.success?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is special because this helper is used inside both its and before_alls, and assertion failures in the latter are not caught. In any case, since the backtick always returns a String, the original should be_truthy never raises, so it was incorrect in the first place.

@straight-shoota straight-shoota added this to the 1.8.0 milestone Feb 23, 2023
@straight-shoota straight-shoota merged commit 94c1f02 into crystal-lang:master Feb 25, 2023
@HertzDevil HertzDevil deleted the bug/expectations-outside-specs branch February 27, 2023 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. kind:specs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants