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

feat: Update instrumentation generator #1094

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .instrumentation_generator/instrumentation_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,19 @@ def add_to_instrumentation_all
insert_into_file("#{instrumentation_all_path}/lib/opentelemetry/instrumentation/all.rb", all_rb_text, after: "# SPDX-License-Identifier: Apache-2.0\n")
end

def update_ci_workflow
ci_file = '.github/workflows/ci-instrumentation.yml'
dependabot_file = '.github/dependabot.yml'

# Update the CI workflow file
insert_into_file(ci_file, " - #{instrumentation_name}\n", after: " gem:\n")

# Update the Dependabot configuration file
insert_into_file(dependabot_file, " - package-ecosystem: 'bundler'\n directory: '/instrumentation/#{instrumentation_name}'\n", after: "updates:\n")

puts "Updated #{ci_file} and #{dependabot_file} successfully."
end

private

def opentelemetry_version
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
# test/instrumentation_generator_test.rb

require 'minitest/autorun'
require 'mocha/minitest'
Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn't able to run these tests locally. When I tried to run the file from the root directory using:

bundle exec ruby .instrumentation_generator/test/instrumentation_generator_test.rb

I got the following error:

/Users/kreopelle/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/mocha-1.16.1/lib/mocha/integration/mini_test/adapter.rb:26:in `included': uninitialized constant MiniTest (NameError)

          Mocha::ExpectationErrorFactory.exception_class = ::MiniTest::Assertion
                                                           ^^^^^^^^^^
Did you mean?  Minitest
        from /Users/kreopelle/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/mocha-1.16.1/lib/mocha/integration/mini_test.rb:50:in `include'
        from /Users/kreopelle/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/mocha-1.16.1/lib/mocha/integration/mini_test.rb:50:in `activate'
        from /Users/kreopelle/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/mocha-1.16.1/lib/mocha/minitest.rb:5:in `<top (required)>'
        from <internal:/Users/kreopelle/.rbenv/versions/3.3.1/lib/ruby/site_ruby/3.3.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
        from <internal:/Users/kreopelle/.rbenv/versions/3.3.1/lib/ruby/site_ruby/3.3.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
        from instrumentation_generator_test.rb:4:in `<main>'

It looks like mocha hasn't updated its call to MiniTest::Assertion to use the Minitest spelling. Minitest used to accept both spellings, but now it only accepts Minitest.

Minitest has its own stubbing features that can be used, though you have to use them in a specific test and can't set them in a setup method. Here's one of my favorite articles on stubbing in Minitest: https://semaphoreci.com/community/tutorials/mocking-in-ruby-with-minitest

When I removed mocha/minitest and took the @generator.stubs method calls out of the setup method, I was able to run the tests.

The only error I got was related to calling puts:

  1) Error:
InstrumentationGeneratorTest#test_update_ci_workflow:
NoMethodError: private method `puts' called for an instance of InstrumentationGenerator
    .instrumentation_generator/test/instrumentation_generator_test.rb:86:in `test_update_ci_workflow'

6 runs, 21 assertions, 0 failures, 1 errors, 0 skips

require 'thor'
require_relative '../instrumentation_generator'

class InstrumentationGeneratorTest < Minitest::Test
def setup
@instrumentation_name = 'new_instrumentation'
@generator = InstrumentationGenerator.new([@instrumentation_name])

@generator.stubs(:template)
@generator.stubs(:insert_into_file)
@generator.stubs(:puts)
end
Comment on lines +9 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

Since running the generator creates files, I think the files should be deleted between the tests to make sure we have a clean slate for each test.

Like the setup method you added, minitest also has a teardown method.

Maybe in teardown we could delete the directory we expect to get created if it exists?


def test_root_files
@generator.root_files

assert @generator.template('templates/rubocop.yml.tt', 'instrumentation/new_instrumentation/.rubocop.yml')
Copy link
Contributor

Choose a reason for hiding this comment

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

First off, thank you for adding tests! It's great to have coverage on a tool that we use whenever we add new instrumentation.

I like this example as the three options a Ruby unit test should evaluate1:

  • It returns a value.
  • It passes the work along to somewhere else (i.e., dispatches the work elsewhere).
  • It causes a side effect.

In this test, it's almost as if we're testing whether the @generator.template method can be called. Since we don't define @generator.template, (that's from Thor), I think we could take a different approach to make sure everything is working as expected.

Of the three options listed above, I think we want to make sure this method causes a side effect.

Since the root_files method is supposed to create these files from the ERB templates, what if we checked to see if the files were created when the method is run?

That could look like:
assert File.exist?('instrumentation/new_instrumentation/.yardopts')

Let me know if you want help brainstorming assertions related to side effects of the other methods.

Footnotes

  1. https://www.cloudbees.com/blog/unit-testing-in-ruby#what-should-be-tested

assert @generator.template('templates/yardopts.tt', 'instrumentation/new_instrumentation/.yardopts')
assert @generator.template('templates/Appraisals', 'instrumentation/new_instrumentation/Appraisals')
assert @generator.template('templates/CHANGELOG.md.tt', 'instrumentation/new_instrumentation/CHANGELOG.md')
assert @generator.template('templates/Gemfile', 'instrumentation/new_instrumentation/Gemfile')
assert @generator.template('templates/LICENSE', 'instrumentation/new_instrumentation/LICENSE')
assert @generator.template('templates/gemspec.tt', 'instrumentation/new_instrumentation/opentelemetry-instrumentation-new_instrumentation.gemspec')
assert @generator.template('templates/Rakefile', 'instrumentation/new_instrumentation/Rakefile')
assert @generator.template('templates/Readme.md.tt', 'instrumentation/new_instrumentation/README.md')
end

def test_lib_files
@generator.lib_files

assert @generator.template('templates/lib/entrypoint.rb', 'instrumentation/new_instrumentation/lib/opentelemetry-instrumentation-new_instrumentation.rb')
assert @generator.template('templates/lib/instrumentation.rb.tt', 'instrumentation/new_instrumentation/lib/opentelemetry/instrumentation.rb')
assert @generator.template('templates/lib/instrumentation/instrumentation_name.rb.tt', 'instrumentation/new_instrumentation/lib/opentelemetry/instrumentation/new_instrumentation.rb')
assert @generator.template('templates/lib/instrumentation/instrumentation_name/instrumentation.rb.tt', 'instrumentation/new_instrumentation/lib/opentelemetry/instrumentation/new_instrumentation/instrumentation.rb')
assert @generator.template('templates/lib/instrumentation/instrumentation_name/version.rb.tt', 'instrumentation/new_instrumentation/lib/opentelemetry/instrumentation/new_instrumentation/version.rb')
end

def test_test_files
@generator.test_files

assert @generator.template('templates/test/test_helper.rb', 'instrumentation/new_instrumentation/test/test_helper.rb')
assert @generator.template('templates/test/instrumentation.rb', 'instrumentation/new_instrumentation/test/opentelemetry/instrumentation/new_instrumentation/instrumentation_test.rb')
end

def test_add_to_releases
@generator.add_to_releases

expected_release_details = <<-HEREDOC
- name: opentelemetry-instrumentation-new_instrumentation
directory: instrumentation/new_instrumentation
version_constant: [OpenTelemetry, Instrumentation, NewInstrumentation, VERSION]\n
HEREDOC

assert @generator.insert_into_file('.toys/.data/releases.yml', expected_release_details.strip, after: "gems:\n")
end

def test_add_to_instrumentation_all
@generator.add_to_instrumentation_all

gemfile_text = "\ngem 'opentelemetry-instrumentation-new_instrumentation', path: '../new_instrumentation'"
gemspec_text = "\n spec.add_dependency 'opentelemetry-instrumentation-new_instrumentation', '~> 0.0.0'"
all_rb_text = "\nrequire 'opentelemetry-instrumentation-new_instrumentation'"

assert @generator.insert_into_file('instrumentation/all/Gemfile', gemfile_text, after: "gemspec\n")
assert @generator.insert_into_file('instrumentation/all/opentelemetry-instrumentation-all.gemspec', gemspec_text, after: "spec.required_ruby_version = '>= 3.0'\n")
assert @generator.insert_into_file('instrumentation/all/lib/opentelemetry/instrumentation/all.rb', all_rb_text, after: "# SPDX-License-Identifier: Apache-2.0\n")
end

def test_update_ci_workflow
@generator.update_ci_workflow

ci_file_text = " - new_instrumentation\n"
dependabot_file_text = " - package-ecosystem: 'bundler'\n directory: '/instrumentation/new_instrumentation'\n"

assert @generator.insert_into_file('.github/workflows/ci-instrumentation.yml', ci_file_text, after: " gem:\n")
assert @generator.insert_into_file('.github/dependabot.yml', dependabot_file_text, after: "updates:\n")

assert @generator.puts('Updated .github/workflows/ci-instrumentation.yml and .github/dependabot.yml successfully.')
end
end
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@ source 'https://rubygems.org'
gem 'rake', '~> 13.0'
gem 'rubocop', '~> 1.64.0'
gem 'rubocop-performance', '~> 1.21.0'
gem 'minitest', '~> 5.14'
gem 'mocha', '~> 1.12', require: false