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

Convert tests to plain go tests #122

Merged
merged 1 commit into from
Nov 16, 2020

Conversation

jpkrohling
Copy link
Member

Signed-off-by: Juraci Paixão Kröhling juraci@kroehling.de

@jpkrohling jpkrohling force-pushed the jpkrohling/113-gotests branch from 037e56f to 1aa4947 Compare November 16, 2020 09:53
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
@jpkrohling jpkrohling force-pushed the jpkrohling/113-gotests branch from 1aa4947 to 741f16a Compare November 16, 2020 09:55
@jpkrohling
Copy link
Member Author

@kearls, @objectiser, could one of you please review this? I'm mostly AFK today, but wanted to open this PR anyway as this might take a while to review.

Most of the changes are quite mechanic and there shouldn't be any changes in behavior. There was one test removed, as it was exactly the same as another existing one.

Copy link
Member

@kevinearls kevinearls left a comment

Choose a reason for hiding this comment

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

Given the length of this PR I did not do an in-depth line by line review, but did a quick pass on almost everything and it looks good to me.

Since you're already using stretchr it might be worth using their suites package for these tests two. I might be able to work on that if you want to open an issue for it.

@jpkrohling
Copy link
Member Author

Since you're already using stretchr it might be worth using their suites package for these tests two

I confess that I don't know stretchr as much as I should, so, I'm not quite sure I understand the benefits. Do you mean to group the envtests ?

@jpkrohling jpkrohling merged commit 0e20605 into open-telemetry:master Nov 16, 2020
@kevinearls
Copy link
Member

@jpkrohling The suites package gives you things you'd typically have in an xUnit type framework, notably beforeSuite, beforeTest, afterTest, and afterSuite functions. If I remember correctly for the Jaeger Operator E2E tests it also made it easier to do reporting.

shree007 pushed a commit to shree007/opentelemetry-operator that referenced this pull request Dec 12, 2021
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
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