Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Move observer_test_helpers.go to a separate package #1335

Merged
merged 3 commits into from
Aug 9, 2023

Conversation

lambdanis
Copy link
Contributor

@lambdanis lambdanis commented Aug 8, 2023

I hit an import cycle in observer_test_helpers.go when working on #1304, so I re-implemented metrics.EnableMetrics() there instead of importing the metrics package. Then I hit the same import cycle when working on #1279, so here I moved observer_test_helpers.go to a separate package.

It's a pretty straightforward refactoring, see commits for details. Technically it is a breaking change, but only developer-facing.

This is needed to refactor observer_test_helpers.go as a separate package.

Signed-off-by: Anna Kapuscinska <anna@isovalent.com>
@lambdanis lambdanis requested a review from a team as a code owner August 8, 2023 19:32
@lambdanis lambdanis requested a review from mtardy August 8, 2023 19:32
Some functions in observer_test_helper.go follow the same logic as Tetragon
daemon. This was problematic because daemon imports packages that import
observer, but Go doesn't allow import cycles. Furthermore, it's conceptually
a different functionality than the rest of the observer package.

This commit moves observer_test_helper.go file to a separate package:
github.com/cilium/tetragon/pkg/observer/observertesthelper. Apart from updating
all necessary imports there are no other changes.

Signed-off-by: Anna Kapuscinska <anna@isovalent.com>
Now that observertesthelper is a separate package, we can import metrics
package there.

Signed-off-by: Anna Kapuscinska <anna@isovalent.com>
@lambdanis lambdanis force-pushed the pr/lambdanis/observer-test-helpers branch from bb3607d to bd51be3 Compare August 8, 2023 19:34
Copy link
Contributor

@jrfastab jrfastab left a comment

Choose a reason for hiding this comment

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

ship it

@kkourt kkourt merged commit 2a1bf1d into cilium:main Aug 9, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants