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

[chore][CI/CD] Remove integration test naming restriction #32529

Merged
merged 22 commits into from
May 2, 2024

Conversation

crobert-1
Copy link
Member

Description:

Including the -run=Integration argument in the make command means that only integration tests that include Integration in their name will be run. This requirement is not obvious to users, so there are currently around 15 integration tests that aren't being run because they don't conform to this requirement. I think the best approach here is to remove the -run argument (rather than rename tests) so that this doesn't happen again.

Link to tracking Issue:
Resolves #32207

@crobert-1 crobert-1 requested review from a team and MovieStoreGuy April 18, 2024 18:44
@crobert-1 crobert-1 marked this pull request as draft April 18, 2024 18:44
@crobert-1 crobert-1 added the Run Windows Enable running windows test on a PR label Apr 18, 2024
@crobert-1
Copy link
Member Author

crobert-1 commented Apr 26, 2024

I've filed #32715 for the failing CI/CD action, it's unrelated to this PR.

@crobert-1 crobert-1 added the ready to merge Code review completed; ready to merge by maintainers label Apr 26, 2024
@crobert-1
Copy link
Member Author

Added ready to merge as multiple project approvers and maintainers have approved. This change spans many components so there aren't any specific code owners here, from what I can tell.

djaglowski pushed a commit that referenced this pull request Apr 30, 2024
…32717)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
This fixes 2/3 failing integration tests in
#32543.
The last failing test `TestAlertsReceiverTLS` needs to be skipped for
now, as its cert files are invalid and need to be re-generated.

All three of these tests are currently being skipped, but will run again
when #32529 is merged.

**Link to tracking Issue:** <Issue number if applicable>

#32529

#32543

**Testing:** <Describe what testing was performed and which tests were
added.>
Two tests are passing again locally.
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Please resolve the conflict, note that the PR that needed to be merged to remove skipping a test has also been merged

@crobert-1
Copy link
Member Author

crobert-1 commented May 1, 2024

I've resolved the conflict and updated other tests based on more merged PRs. This should be ready (pending passing CI/CD).

@codeboten codeboten merged commit 33b1573 into open-telemetry:main May 2, 2024
178 checks passed
@github-actions github-actions bot added this to the next release milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI/CD] Some integration tests are currently being skipped due to naming restriction
7 participants