Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.

Test build#3055

Closed
JonRowe wants to merge 1 commit intomainfrom
test-build
Closed

Test build#3055
JonRowe wants to merge 1 commit intomainfrom
test-build

Conversation

@JonRowe
Copy link
Member

@JonRowe JonRowe commented Dec 4, 2023

No description provided.

@pirj
Copy link
Member

pirj commented Dec 4, 2023

/home/pirj/source/rspec-expectations/lib/rspec/matchers/dsl.rb:75: warning: method redefined; discarding old exist
/home/pirj/source/rspec-expectations/lib/rspec/matchers.rb:596: warning: previous definition of exist was here

@pirj
Copy link
Member

pirj commented Dec 4, 2023

@JonRowe JonRowe closed this Dec 4, 2023
@JonRowe JonRowe deleted the test-build branch December 4, 2023 22:42
@JonRowe
Copy link
Member Author

JonRowe commented Dec 4, 2023

Yes I agree! I've been working on a fix, basically just rewriting rspec-rails tests to not need that matcher.

@pirj
Copy link
Member

pirj commented Dec 4, 2023

Wrapping the file path string into something that

- subject(:generator_spec) { file("spec/generator/posts_generator_spec.rb") }
+ subject(:generator_spec) { Pathname.new(file("spec/generator/posts_generator_spec.rb")) }

fixes those issues.

But I think this would be better alexrothenberg/ammeter#70

@JonRowe
Copy link
Member Author

JonRowe commented Dec 5, 2023

That PR will break our specs again as expect a string in places, did you check what effect that has on the contain matcher which we use extensively?

@pirj
Copy link
Member

pirj commented Dec 5, 2023

That’s weird. It was late night but I recall making that change in a live ammeter directory and that make our rspec-rails spec run green. I didn’t check cucumber though.
I didn’t think about the compatibility of your change and my ammeter patch though.
Let me check in the evening.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants