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

configure mockito agent #11560

Merged

Conversation

yannicklamprecht
Copy link
Contributor

@yannicklamprecht yannicklamprecht requested a review from a team as a code owner November 1, 2024 23:47
Copy link
Member

@electronicboy electronicboy left a comment

Choose a reason for hiding this comment

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

maybe somebody more in-tune with gradle will flag this, but, this LGTM

Copy link
Member

@jpenilla jpenilla left a comment

Choose a reason for hiding this comment

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

This should use a CommandLineArgumentProvider to avoid resolving dependencies during configuration time - I think there are some other spots that may need to change in this direction too, for example stuff used by Javadoc, but it would be better to not increase the tech debt here.

@yannicklamprecht yannicklamprecht force-pushed the feature/configure-mockito-agent branch from 8346235 to ac66afc Compare November 5, 2024 19:53
@yannicklamprecht
Copy link
Contributor Author

This should use a CommandLineArgumentProvider to avoid resolving dependencies during configuration time - I think there are some other spots that may need to change in this direction too, for example stuff used by Javadoc, but it would be better to not increase the tech debt here.

Hopefully addressed that to your liking.

@jpenilla
Copy link
Member

jpenilla commented Nov 5, 2024

It should be something like this

abstract class MockitoAgentProvider : CommandLineArgumentProvider {
  @get:InputFiles
  abstract val fileCollection: ConfigurableFileCollection

  override fun asArguments(): Iterable<String> {
    return listOf("..." + fileCollection.files.single().absolutePath)
  }
}
tasks.test {
  val provider = objects.newInstance<MockitoAgentProvider>()
  provider.fileCollection.from(configuration)
  jvmArgumentProviders.add(provider)
}

also should use register instead of create for the Configuration

@yannicklamprecht yannicklamprecht force-pushed the feature/configure-mockito-agent branch 2 times, most recently from e938140 to 52dcbfb Compare November 5, 2024 22:35
patches/api/0003-Test-changes.patch Outdated Show resolved Hide resolved
@yannicklamprecht yannicklamprecht force-pushed the feature/configure-mockito-agent branch from 52dcbfb to 5ccbc1b Compare November 5, 2024 23:09
Copy link
Member

@jpenilla jpenilla left a comment

Choose a reason for hiding this comment

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

Looks fine, but one thing to consider is if Spigot configures the equivalent in Maven then this should move to the setup gradle build patch.

@yannicklamprecht yannicklamprecht force-pushed the feature/configure-mockito-agent branch from 5ccbc1b to 0428003 Compare November 9, 2024 21:12
@lynxplay lynxplay merged commit e47f79a into PaperMC:master Nov 9, 2024
3 checks passed
@yannicklamprecht yannicklamprecht deleted the feature/configure-mockito-agent branch November 9, 2024 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

5 participants