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

Add JUnit annotate BK plugin #5473

Merged
merged 31 commits into from
Oct 8, 2024

Conversation

alexsapran
Copy link
Contributor

@alexsapran alexsapran commented Sep 9, 2024

What does this PR do?

This PR adds the JUnit annotate Buildkite plugin

Why is it important?

This is goint to help with easier triaging what failed on each build.

I pushed a temp commit to cause some errors. See the screenshot
Screenshot 2024-10-07 at 1 53 12 PM

Now, when a test fails, you see the failure at the top with an additional link to the step that caused that error.

Example build with a failure to show this in action https://buildkite.com/elastic/elastic-agent/builds/12866

How to test this PR locally

This is a Buildkite change, only testable on BK.

Related issues

Related: https://github.com/elastic/ingest-dev/issues/3773

Copy link
Contributor

mergify bot commented Sep 9, 2024

This pull request does not have a backport label. Could you fix it @alexsapran? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

agents:
provider: "gcp"
depends_on:
- step: "unit-tests-2204"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed something that might cause a problem.
Some steps produce the same artifact for reporting the test files, for example, unit-tests-2204 produce the same xml files as unit-tests-2204-arm64

fileName := fmt.Sprintf("build/TEST-go-%s", strings.Replace(strings.ToLower(name), " ", "_", -1))
fileName := ""
_, isCI := os.LookupEnv("CI")
if isCI {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When running inside CI, the BK plugin will try to link the failure to the step if the generated file has this pattern.

see: https://github.com/buildkite-plugins/junit-annotate-buildkite-plugin#job-uuid-file-pattern-optional

Copy link
Contributor

mergify bot commented Sep 10, 2024

backport-v8.x has been added to help with the transition to the new branch 8.x.

Copy link
Contributor

mergify bot commented Sep 11, 2024

backport-v8.x has been added to help with the transition to the new branch 8.x.

@mergify mergify bot added the backport-8.x Automated backport to the 8.x branch with mergify label Sep 11, 2024
@v1v v1v removed the backport-v8.x label Sep 11, 2024
@ycombinator ycombinator force-pushed the bk/add-bk-plugin-junit-annotate branch from 967b239 to c77020b Compare September 20, 2024 00:42
This is goint to help with easier triaging what failed on each build.

Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Because some steps are producing the same artifact names, when uploaded
and re-downloaed they overwrite each other.
This is why we are attempting to split the annotate after each step to
make it easier to deferentiate between the artifacts from each step

Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
When running inside BK, we are running the same commands but in
different enviroments, this PR changes this and differentiate each
resulting artifact of JUnit to have a unique name for the step that was
generated.
This in combination with the JUnit annotate plugin provide better
understanding on what failed on which step in the build.

Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
@alexsapran alexsapran force-pushed the bk/add-bk-plugin-junit-annotate branch from c77020b to 98bfbfd Compare October 3, 2024 10:33
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Since we are renaming the cov file for each step to use the same name,
lets move it instead and spare the traffic of one more file

Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
source .buildkite/scripts/common.sh
set +euo pipefail
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We set pipefail in the common.sh so the script was never run and performed the proper coverage files.

Copy link
Contributor

Choose a reason for hiding this comment

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

What script was never run?

Copy link
Contributor Author

@alexsapran alexsapran Oct 8, 2024

Choose a reason for hiding this comment

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

Because of the order in which we set the bash flags, the' pipe failwas overridden inside thecommon.sh`, so the exit code's wrapping was never run.

So if the mage command failed, the script would exit, not producing any coverage.out but only a Test-go-unit.cov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

coincidently, this failure is seen in this unrelated build https://buildkite.com/elastic/elastic-agent/builds/12911#_

Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@alexsapran alexsapran marked this pull request as ready for review October 8, 2024 09:36
@alexsapran alexsapran requested a review from a team as a code owner October 8, 2024 09:36
@swiatekm swiatekm removed their request for review October 8, 2024 13:55
@alexsapran alexsapran merged commit 0d5b335 into elastic:main Oct 8, 2024
15 checks passed
@alexsapran alexsapran deleted the bk/add-bk-plugin-junit-annotate branch October 8, 2024 14:09
mergify bot pushed a commit that referenced this pull request Oct 8, 2024
* Add JUnit annotate BK plugin

Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
(cherry picked from commit 0d5b335)
pierrehilbert pushed a commit that referenced this pull request Oct 9, 2024
* Add JUnit annotate BK plugin

Signed-off-by: Alexandros Sapranidis <alexandros@elastic.co>
(cherry picked from commit 0d5b335)

Co-authored-by: Alexandros Sapranidis <alexandros@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8.x Automated backport to the 8.x branch with mergify ci skip-changelog Team:Ingest-EngProd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants