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

Test build #3055

Closed
wants to merge 1 commit into from
Closed

Test build #3055

wants to merge 1 commit into from

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

@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 join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants