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

Improve the problem checking for the daemon-reuse test #29057

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hegyibalint
Copy link
Member

I found this test, which could use some help when it comes to problem checking.

In addition, I've made the buildOperationFixture protected, so other tests could fetch the build operations coming in without the need to make the fixture themselves.

@hegyibalint hegyibalint added a:chore Minor issue without significant impact in:java-plugins java-library, java, java-base, java-platform, java-test-fixtures labels May 8, 2024
@hegyibalint hegyibalint requested a review from a team May 8, 2024 10:56
@hegyibalint hegyibalint self-assigned this May 8, 2024
@hegyibalint hegyibalint requested review from a team as code owners May 8, 2024 10:56
@hegyibalint hegyibalint requested a review from tresat May 8, 2024 10:56
@hegyibalint hegyibalint changed the title Improve the problem checking Improve the problem checking for the daemon-reuse test May 8, 2024
@hegyibalint hegyibalint added this pull request to the merge queue May 9, 2024
@bot-gradle bot-gradle added this to the 8.9 RC1 milestone May 9, 2024
@bot-gradle
Copy link
Collaborator

The merge queue build has failed. Click here to see all failures.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 9, 2024
@@ -116,7 +116,8 @@ class JavaCompilerDaemonReuseIntegrationTest extends AbstractCompilerDaemonReuse
}

def "log messages from a compiler daemon are associated with the task that generates them"() {
def buildOperations = new BuildOperationsFixture(executer, temporaryFolder)
enableBuildOperationsFixture()
Copy link
Member

Choose a reason for hiding this comment

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

❌ It seems redundant as calling enableProblemsApiCheck() also calls enableBuildOperationsFixture()

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to keep it redundant, as removing the enableProblemsApiCheck() would remove the build fixture. This is more about telling the reader that we need that fixture.

@@ -65,7 +65,7 @@ class KnownProblemIds {
'compilation:java:java-compilation-error' : 'Java compilation error',
'compilation:java:java-compilation-failed' : 'Java compilation error',
'compilation:java:java-compilation-warning' : 'Java compilation warning',
'compilation:java:java-compilation-advice' : 'Java compilation note',
'compilation:java:java-compilation-note' : 'Java compilation note',
Copy link
Member

Choose a reason for hiding this comment

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

🤔 maybe compilation:java:java-compilation-info, or compilation:java:java-compilation-advice?

Copy link
Member Author

Choose a reason for hiding this comment

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

@hegyibalint hegyibalint requested a review from donat May 10, 2024 10:12
@hegyibalint hegyibalint added this pull request to the merge queue May 14, 2024
@bot-gradle
Copy link
Collaborator

The merge queue build has failed. Click here to see all failures.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 min review a:chore Minor issue without significant impact in:java-plugins java-library, java, java-base, java-platform, java-test-fixtures
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants