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

describe :symbol causes the subject to be "symbol", not :symbol #1240

Closed
myronmarston opened this issue Jan 4, 2014 · 5 comments · Fixed by #1403
Closed

describe :symbol causes the subject to be "symbol", not :symbol #1240

myronmarston opened this issue Jan 4, 2014 · 5 comments · Fixed by #1403
Milestone

Comments

@myronmarston
Copy link
Member

Discovered by @sferik in rspec/rspec-expectations#405.

@JonRowe
Copy link
Member

JonRowe commented Jan 5, 2014

I'd expect that behaviour, as we generally expect it to be a string or class, not an instance which I guess a symbol is. I suppose we could special case special primitives but is it worth it?

@sferik
Copy link

sferik commented Jan 5, 2014

It seems to work for other instances, just not symbols:

obj = Object.new

describe obj do
  it { expect(subject).to eq(obj) }
end

Can you point to the specific code that’s causing this bug (i.e. where the special case would need to be inserted)? I’m new to the codebase and am not quite sure where this is happening.

@myronmarston
Copy link
Member Author

I'd expect that behaviour, as we generally expect it to be a string or class, not an instance which I guess a symbol is. I suppose we could special case special primitives but is it worth it?

Classes and strings are most common, but most other built-in types work. For example, the following all work:

describe 3 do
  it { is_expected.to be_between(2, 4) }
end

describe 3.1 do
  it { is_expected.to be_a(Float) }
end

describe ["foo"] do
  it { is_expected.to eq ["foo"] }
end

describe (1..10) do
  it { is_expected.to be_a(Range) }
end

describe Date.today do
  it { is_expected.to be_a(Date) }
end

describe Time.now do
  it { is_expected.to be_a(Time) }
end

It looks like hashes don't, however:

describe({:foo => 3, :bar => 2}) do
  it { is_expected.to be_a(Hash) }
end

...fails with expected "" to be a kind of Hash. I suspect that may have to do with it interpretting the hash as metadata.

@sferik -- implicit subject is defined in memoized_helpers.rb. That in turn uses described_class or description, which is defined to get their values from metadata, which turns out to have a special case for symbols.

@sferik
Copy link

sferik commented Jan 5, 2014

IMHO, the correct solution is to remove the || Symbol === candidate special case in RSpec 3.

Perhaps it even makes sense to remove GroupMetadataHash altogether?

@JonRowe
Copy link
Member

JonRowe commented Jan 5, 2014

Interesting, I'd guess it's to maintain the Symbol / String parity used elsewhere. (E.g. Symbols are often used as frozen/special strings). I see no harm in removing this though...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants