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] make contrib tests more efficient #12490

Merged
merged 4 commits into from
Feb 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 26 additions & 4 deletions .github/workflows/contrib-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,26 @@ concurrency:
permissions: read-all

jobs:
contrib-tests-prepare:
runs-on: ubuntu-latest
if: ${{ !contains(github.event.pull_request.labels.*.name, 'Skip Contrib Tests') }}
steps:
- name: Checkout Repo
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
- name: Prepare Contrib Tests
run: |
contrib_path=/tmp/opentelemetry-collector-contrib
git clone --depth=1 https://github.com/open-telemetry/opentelemetry-collector-contrib.git $contrib_path
make CONTRIB_PATH=$contrib_path prepare-contrib
- uses: actions/upload-artifact@v4
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't it mean that we will be testing against one contrib version for 90 days?

Copy link
Member

@dmitryax dmitryax Feb 28, 2025

Choose a reason for hiding this comment

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

I'm looking at https://github.com/actions/upload-artifact?tab=readme-ov-file#retention-period. Should we add git sha in the path ?

Copy link
Contributor Author

@atoulme atoulme Feb 28, 2025

Choose a reason for hiding this comment

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

ouch. I'll open a new PR to add that
Actually, no, it might not be a problem, since this happens every time and is used inside the same build. But we can certainly do it anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to upload it then? Should we use https://github.com/actions/cache instead?

Copy link
Contributor Author

@atoulme atoulme Feb 28, 2025

Choose a reason for hiding this comment

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

I followed the recipe I have seen used most of the time. A cache is a step better I didn't look into because I was afraid of having cache issues. That would shave an additional 4 minutes off the build.

with:
name: contrib
path: /tmp/opentelemetry-collector-contrib/
include-hidden-files: true

contrib-tests-matrix:
runs-on: ubuntu-latest
needs: [contrib-tests-prepare]
if: ${{ !contains(github.event.pull_request.labels.*.name, 'Skip Contrib Tests') }}
strategy:
fail-fast: false
Expand All @@ -40,16 +58,20 @@ jobs:
steps:
- name: Checkout Repo
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
- name: Download contrib
uses: actions/download-artifact@v4
with:
name: contrib
path: /tmp/contrib
- name: Setup Go
uses: actions/setup-go@f111f3307d8850f501ac008e886eec1fd1932a34 # v5.3.0
with:
go-version: ~1.23.6
cache: false
- name: Run Contrib Tests
- name: Run tests
run: |
contrib_path=/tmp/opentelemetry-collector-contrib
git clone --depth=1 https://github.com/open-telemetry/opentelemetry-collector-contrib.git $contrib_path
make CONTRIB_PATH=$contrib_path SKIP_RESTORE_CONTRIB=true GROUP=${{ matrix.group }} check-contrib
chmod +x /tmp/contrib/.tools/*
make CONTRIB_PATH=/tmp/contrib SKIP_RESTORE_CONTRIB=true GROUP=${{ matrix.group }} check-contrib

contrib_tests:
runs-on: ubuntu-latest
Expand Down
10 changes: 6 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -261,17 +261,19 @@ gensemconv: $(SEMCONVGEN) $(SEMCONVKIT)

ALL_MOD_PATHS := "" $(ALL_MODULES:.%=%)

# Checks that the HEAD of the contrib repo checked out in CONTRIB_PATH compiles
# against the current version of this repo.
.PHONY: check-contrib
check-contrib:
.PHONY: prepare-contrib
prepare-contrib:
@echo Setting contrib at $(CONTRIB_PATH) to use this core checkout
@$(MAKE) -j2 -C $(CONTRIB_PATH) for-all CMD="$(GOCMD) mod edit \
$(addprefix -replace ,$(join $(ALL_MOD_PATHS:%=go.opentelemetry.io/collector%=),$(ALL_MOD_PATHS:%=$(CURDIR)%)))"
@$(MAKE) -j2 -C $(CONTRIB_PATH) gotidy

@$(MAKE) generate-contrib

# Checks that the HEAD of the contrib repo checked out in CONTRIB_PATH compiles
# against the current version of this repo.
.PHONY: check-contrib
check-contrib:
@echo -e "\nRunning tests"
@$(MAKE) -C $(CONTRIB_PATH) gotest

Expand Down
Loading