Skip to content

test: continue switch to rspec-mocks#2059

Open
thompson-tomo wants to merge 13 commits intoopen-telemetry:mainfrom
thompson-tomo:patch-10
Open

test: continue switch to rspec-mocks#2059
thompson-tomo wants to merge 13 commits intoopen-telemetry:mainfrom
thompson-tomo:patch-10

Conversation

@thompson-tomo
Copy link
Copy Markdown
Contributor

@thompson-tomo thompson-tomo commented Mar 8, 2026

This continues the work in #2011

For Reviewers

Minitests stub method used a block to maintain the scope of the invocation. Switching to rspec mocks resulted in the block being removed and the white space changing in many places in the code so I recommend that you review this using "Hide the white space changes": https://github.com/open-telemetry/opentelemetry-ruby-contrib/pull/2059/changes?w=1

@arielvalentin
Copy link
Copy Markdown
Contributor

Thanks for doing this.

Something I'm trying to understand is if @open-telemetry/ruby-contrib-mainters are in agreement that we converge on using rspec mocks, or if we should exclusively use minitest

@thompson-tomo
Copy link
Copy Markdown
Contributor Author

thompson-tomo commented Mar 12, 2026

No worries, I was just looking to unblock the renovate pr for minitest by replicating your previous work. 😉 hence the other pr #2061.

Copy link
Copy Markdown
Member

@robbkidd robbkidd left a comment

Choose a reason for hiding this comment

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

While we figure out minitest vs rspec, I found one thing that needs looking into.

Co-authored-by: Robb Kidd <robb@thekidds.org>
@robbkidd
Copy link
Copy Markdown
Member

This test [for instrumentation-rake] fails on my machine. Not sure why it didn't fail in CI.

Er, I know why now. instrumentation-rake isn't named in the CI test matrix. 😬

Git spelunking turns up that the rake instrumentation was added around the same time as the old single CI definition was split into different workflows. I'm pretty sure we missed adding rake to the test matrix. 😅

We should update ci-instrumentation-full (rake needs no services):

@thompson-tomo, would you be up for including that CI update in this PR? 🥺

@thompson-tomo
Copy link
Copy Markdown
Contributor Author

@robbkidd thanks for looking into that, i have made the above changes in #2099 to avoid it being held up in rspec-mocks vs minitest discussion.

@robbkidd robbkidd dismissed their stale review March 20, 2026 22:32

The change requested was done.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants