Skip to content

[wip] try to fix test names #5259

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.

Sign up for GitHub

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

Draft
cbeauchesne wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

[wip] try to fix test names #5259

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
69d1bdb
[wip] try to fix test names
cbeauchesne Jan 20, 2026
929a272
Second try
cbeauchesne Jan 20, 2026
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
15 changes: 11 additions & 4 deletions spec/datadog/tracing/contrib/extensions_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,17 @@
let(:integration_name) { :example }
let(:integration) { integration_class.new(integration_name) }
let(:integration_class) do
Class.new do
include Datadog::Tracing::Contrib::Integration
include Datadog::Tracing::Contrib::Configurable
end
stub_const(
'TestIntegrationClass',
Comment on lines +11 to +12
Copy link
Member

@lloeki lloeki Jan 20, 2026

Choose a reason for hiding this comment

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

Looks like this is a heavy-handed way to have the class's name method return TestIntegrationClass or something.

Is anything even referencing the constant TestIntegrationClass anywhere?

Copy link
Contributor Author

@cbeauchesne cbeauchesne Jan 20, 2026

Choose a reason for hiding this comment

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

Is anything even referencing the constant TestIntegrationClass anywhere?

No, it's only a test to see if it fixes the issue. I'll definitly need your help to arrange this in a way that fit ruby idioms.

Class.new do
include Datadog::Tracing::Contrib::Integration
include Datadog::Tracing::Contrib::Configurable

def inspect
'TestIntegrationClass'
end
Comment on lines +17 to +19
Copy link
Member

@lloeki lloeki Jan 20, 2026

Choose a reason for hiding this comment

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

This is one of the ways to remove the standard #<Class:0x12345> stringification of dynamic classes.

There's also to_s that might end up being used, and self.name. Maybe self.to_s and self.inspect as well.

Copy link
Contributor Author

@cbeauchesne cbeauchesne Jan 20, 2026

Choose a reason for hiding this comment

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

Which one you think is the best fit ?

Copy link
Member

@p-datadog p-datadog Jan 20, 2026

Choose a reason for hiding this comment

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

You can define classes the old-fashioned way: https://github.com/DataDog/dd-trace-rb/blob/master/spec/datadog/di/serializer_spec.rb#L5

But, I don't know what you are attempting to achieve with this PR.

end
)
end

let(:configurable_module) do
Expand Down