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

add test cases to sequel gem implementation #412

Closed
wants to merge 7 commits into from

Conversation

arjun-rajappa
Copy link
Contributor

Adding support for a new gem - Sequel
Add test cases

@arjun-rajappa arjun-rajappa self-assigned this Aug 14, 2024
@arjun-rajappa arjun-rajappa requested a review from a team August 14, 2024 05:10
@@ -0,0 +1,115 @@
# (c) Copyright IBM Corp. 2024
# (c) Copyright Instana Inc. 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need Instana Inc. on new files anymore.

gemfiles/sequel_50.gemfile Outdated Show resolved Hide resolved
test/instrumentation/sequel_test.rb Show resolved Hide resolved
db_url = ENV['DATABASE_URL'].sub("sqlite3", "sqlite")
@db = Sequel.connect(db_url)

DummyMigration.apply(@db, :up)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the addedd benefit of using this DummyMigration class over simply creating the table?

@db.create_table :blocks do
      String :name
      String :color
end

assert_equal 2, spans.length
span = find_first_span_by_name(spans, :sequel)
data = span[:data][:sequel]
assert data[:sql].start_with?('INSERT INTO')
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also be checked: assert_equal 0, span[:ec]
Furthermore we should also ensure that the insert was successful.


def test_raw
Instana::Tracer.start_or_continue_trace(:sequel_test, {}) do
@db.execute('SELECT 1')
Copy link
Contributor

Choose a reason for hiding this comment

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

@db.execute is not even publicly documented. According to the docs, @db.run is the idiomatic way to execute arbitrary statements.

And while I am no SQL expert, I don't think it is particularly likely that customers would write such queries, I think more likely scenarios, that they query the CURRENT_TIMESTAMP number, or a random() number;

assert_equal 2, spans.length
span = find_first_span_by_name(spans, :sequel)
data = span[:data][:sequel]
assert 'SELECT 1', data[:sql]
Copy link
Contributor

Choose a reason for hiding this comment

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

assert_equal 0, span[:ec]. Was the op successful?

test/instrumentation/sequel_test.rb Outdated Show resolved Hide resolved
spans = ::Instana.processor.queued_spans
assert_equal 2, spans.length
span = find_first_span_by_name(spans, :sequel)

Copy link
Contributor

Choose a reason for hiding this comment

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

We should check that the execution has indeed failed.

def test_raw_error
assert_raises Sequel::DatabaseError do
Instana::Tracer.start_or_continue_trace(:sequel_test, {}) do
@db.execute('INVALID')
Copy link
Contributor

Choose a reason for hiding this comment

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

@db.execute is not even publicly documented. According to the docs, @db.run is the idiomatic way to execute arbitrary statements.

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