opensearch-project / opensearch-build-libraries

Apache License 2.0
6 stars 25 forks source link

Creating Issue Reports for Flaky Test Failures in Gradle Check #436

Closed prudhvigodithi closed 3 months ago

prudhvigodithi commented 3 months ago

Description

Gradle Check flaky test failures issue creation

Issues Resolved

Part of https://github.com/opensearch-project/OpenSearch/issues/13950

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.

prudhvigodithi commented 3 months ago

Adding @dblock since a similar setup was done in past for code inside src/jenkins. I have added a package gradlecheck that has the code which can be used by the library gradleCheckFlakyTestIssueCreate.groovy and jenkinsfile can simply load and call this library. The only change is I did not add implements Serializable also unlike each class part of src/jenkins the code inside src/gradlecheck is used only only gradleCheckFlakyTestIssueCreate library, I have done this way to make sure the code is organized following OOP methods rather than just dumping all the lines under one library inside var/gradleCheckFlakyTestIssueCreate.

Having this now the unit tests I have used mockito-core for testing the code inside tests/gradlecheck and works fine (example ./gradlew test --tests=OpenSearchMetricsQueryTest), but when invoked with existing tests the old tests times out with the following error.

2024-06-11T06:44:05.566+0000 [DEBUG] [org.gradle.cache.internal.DefaultFileLockManager] Lock acquired on daemon addresses registry.
2024-06-11T06:44:05.566+0000 [DEBUG] [org.gradle.cache.internal.DefaultFileLockManager] Releasing lock on daemon addresses registry.
2024-06-11T06:44:05.567+0000 [DEBUG] [org.gradle.cache.internal.DefaultFileLockManager] Waiting to acquire shared lock on daemon addresses registry.
2024-06-11T06:44:05.567+0000 [DEBUG] [org.gradle.cache.internal.DefaultFileLockManager] Lock acquired on daemon addresses registry.
2024-06-11T06:44:05.567+0000 [DEBUG] [org.gradle.cache.internal.DefaultFileLockManager] Releasing lock on daemon addresses registry.

Just deleting the directory src/gradlecheck and running the existing test example ./gradlew clean test --tests=TestUploadTestResults -info -Ppipeline.stack.write=true again moves forward without any timeout error.

I'm wondering why just adding one extra package src/gradlecheck and testing this with existing test suite is not possible ? @gaiksaya @peterzhuamazon @rishabh6788 @getsaurabh02

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 86.36364% with 18 lines in your changes missing coverage. Please review.

Project coverage is 86.84%. Comparing base (cfba629) to head (1da3059).

:exclamation: Current head 1da3059 differs from pull request most recent head 3024899

Please upload reports for the commit 3024899 to get more accurate results.

Files Patch % Lines
...adlecheck/FetchPostMergeFailedTestClassTest.groovy 69.23% 1 Missing and 3 partials :warning:
...radlecheck/FetchPostMergeFailedTestNameTest.groovy 76.47% 1 Missing and 3 partials :warning:
...dlecheck/FetchPostMergeTestGitReferenceTest.groovy 73.33% 1 Missing and 3 partials :warning:
tests/gradlecheck/FetchTestPullRequestsTest.groovy 73.33% 1 Missing and 3 partials :warning:
...c/gradlecheck/FetchPostMergeFailedTestClass.groovy 91.66% 0 Missing and 1 partial :warning:
tests/gradlecheck/CreateMarkDownTableTest.groovy 88.88% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #436 +/- ## ============================================ - Coverage 87.12% 86.84% -0.28% - Complexity 31 60 +29 ============================================ Files 88 100 +12 Lines 233 365 +132 Branches 12 25 +13 ============================================ + Hits 203 317 +114 - Misses 22 26 +4 - Partials 8 22 +14 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

prudhvigodithi commented 3 months ago

Coming from my above comment the hanging tests issue is due to inheritance used in the src/gradlecheck code files, even though this works in runtime and able to complete the end requirement, this hangs during testing. However with the same goal of using src/gradlecheck I have modified the code so that its compatible during testing and does not break the existing tests still honoring fetching the required classes from src/gradlecheck for the gradleCheckFlakyTestChecker library.

I have also modified the library vars/createGithubIssue that will be used in gradleCheckFlakyTestChecker to create/update the issue body with --body-file and also allow to edit the issue body (not just comment), this change broke majority of the tests that use createGithubIssue directly or indirectly.

prudhvigodithi commented 3 months ago

The CI checks are now green and library is tested as jenkins run, sample issues that are created https://github.com/prudhvigodithi/OpenSearch/issues/25 https://github.com/prudhvigodithi/OpenSearch/issues/24.

rishabh6788 commented 3 months ago

The CI checks are now green and library is tested as jenkins run, sample issues that are created prudhvigodithi/OpenSearch#25 prudhvigodithi/OpenSearch#24.

I see that there are two comments, if I remember correctly wasn't it supposed to be just one comment that would keep getting updated or description? Also, I feel the way data is being presented can be improved.

prudhvigodithi commented 3 months ago

I see that there are two comments, if I remember correctly wasn't it supposed to be just one comment that would keep getting updated or description? Also, I feel the way data is being presented can be improved.

Ya I have removed the issueEdit option to test the default behaviour (comment on an open issue rather than editing the body) of the createGithubIssue library. I have added it back.

The markdown table is coming from the approval https://github.com/opensearch-project/OpenSearch/issues/13950#issuecomment-2155221675, this has all the information per commit, associated PR, jenkins build and failed tests. We can always improve based on the feedback once we see this in action by start creating the issues.

rishabh6788 commented 3 months ago

One thought I have is once the workflow has created an issue, and someone works on it, may be updates the comment or description and strikes-through a few failing tests that have been fixed. Now when the workflow runs once again, wouldn't it override the edits made by operator? Are we assuming that operator would only be adding new comments on the issue and not editing the comment/description added by with workflow?

prudhvigodithi commented 3 months ago

One thought I have is once the workflow has created an issue, and someone works on it, may be updates the comment or description and strikes-through a few failing tests that have been fixed. Now when the workflow runs once again, wouldn't it override the edits made by operator? Are we assuming that operator would only be adding new comments on the issue and not editing the comment/description added by with workflow?

AFAIK only maintainers/admins's can edit the issue created by external users and I dont think one can edit the issue created by other user.

prudhvigodithi commented 3 months ago

FYI I have added a comment to an open issue https://github.com/jenkinsci/JenkinsPipelineUnit/issues/595#issuecomment-2164014788 on why we cant test the library directly gradleCheckFlakyTestChecker that has the external classes. Here for our case the gradleCheckFlakyTestChecker just uses the code part of src/gradlecheck, for all the code part of src/gradlecheck is tested and covered in tests/gradlecheck (for reference please check the CodeCov https://github.com/opensearch-project/opensearch-build-libraries/pull/436#issuecomment-2161461640).

prudhvigodithi commented 3 months ago

Thanks @rishabh6788 just pushed a commit change the repo from my fork to repoUrl: "https://github.com/opensearch-project/OpenSearch",.

opensearch-trigger-bot[bot] commented 3 months ago

The backport to 6.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-6.x 6.x
# Navigate to the new working tree
pushd ../.worktrees/backport-6.x
# Create a new branch
git switch --create backport/backport-436-to-6.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 2e95cddcdf42ea02f45025b9ff1941240d7ba82f
# Push it to GitHub
git push --set-upstream origin backport/backport-436-to-6.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-6.x

Then, create a pull request where the base branch is 6.x and the compare/head branch is backport/backport-436-to-6.x.