Skip to content

fix: Limiting gem & tests to mri - #2429

Open
thompson-tomo wants to merge 12 commits into
open-telemetry:mainfrom
thompson-tomo:disable_tests_on_jruby
Open

fix: Limiting gem & tests to mri#2429
thompson-tomo wants to merge 12 commits into
open-telemetry:mainfrom
thompson-tomo:disable_tests_on_jruby

Conversation

@thompson-tomo

Copy link
Copy Markdown
Contributor

This limits the gems to only be used on mri as they require native c code & no java replacements are available as drop-in replacements.

@kaylareopelle kaylareopelle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few questions/small changes. There's also a merge conflict.

Comment thread instrumentation/CONTRIBUTING.md Outdated
Comment thread instrumentation/CONTRIBUTING.md Outdated
Comment thread instrumentation/CONTRIBUTING.md Outdated
Comment on lines +139 to +147
run: |
files=$(ruby -e 'puts Dir["coverage/**/.resultset.json"]')

if [ -z "$files" ]; then
echo "No coverage files found; skipping coverage check for this runtime."
exit 0
fi

bundle exec ruby -e 'require "simplecov"; SimpleCov.minimum_coverage(ENV["MINIMUM_COVERAGE"].to_i); SimpleCov.collate Dir["coverage/**/.resultset.json"];'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this related to the JRuby changes? I'm fine keeping it in this PR, just want to make sure I'm not misunderstanding what it's doing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was necessary to handle when jruby tests were skipped hence the step had no coverage files. Without it the ci errored.

Co-authored-by: Kayla Reopelle <87386821+kaylareopelle@users.noreply.github.com>
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.

2 participants