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.

Sign up for GitHub

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

Jump to bottom

Allow customiztion before command executes. #27

Closed
wants to merge 2 commits into from
Closed

Allow customiztion before command executes. #27

wants to merge 2 commits into from

Conversation

blindpirate
Copy link

@blindpirate blindpirate commented Aug 31, 2018 •
edited
Loading

Context

As an end user, sometimes I want to provide some extra arguments to the commands to be executed by exemplar, for example, in gradle/gradle we want to add -Dorg.gradle.internal.plugins.portal.url.override=<OurOwnRepositoryMirror>. Currently it's not supported by exemplar - exemplar will only run the commands declared in conf files, there's no way to customize them.

This PR adds an annotation @CommandCustomizers to allow users to customize their Commands before execution. A typical usage is:

@RunWith(GradleSamplesRunner.class)
@SamplesRoot("src/test/samples/customization")
@CommandCustomizers(ExtraTaskRequestCustomizer.class)
public class CustomizedExecutionIntegrationTest {
}

public class ExtraTaskRequestCustomizer implements CommandCustomizer {
    @Override
    public Command customize(Command command) {
        List<String> args = new ArrayList<>(command.getArgs());
        args.add("extraArgument");
        return CommandBuilder.fromCommand(command).setArgs(args).build();
    }
}

@eriwen Can you please take a look?

@blindpirate blindpirate added the enhancement New feature or request label Aug 31, 2018
blindpirate
blindpirate commented Aug 31, 2018
View reviewed changes
buildSrc/src/main/kotlin/Dependencies.kt
@@ -19,4 +21,6 @@ object Libraries {
val SLF4J = "org.slf4j:slf4j-simple:${Versions.SLF4J}"
val SPOCK_CORE = "org.spockframework:spock-core:${Versions.SPOCK_CORE}"
val TYPESAFE_CONFIG = "com.typesafe:config:${Versions.TYPESAFE_CONFIG}"
val CGLIB = "cglib:cglib-nodep:${Versions.CGLIB}"
val OBJENESIS = "org.objenesis:objenesis:${Versions.OBJENESIS}"
Copy link
Author

Choose a reason for hiding this comment

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

These two libraries are necessary for spock Mock().

@blindpirate
Allow customiztion before command executes.
2a64083 
Previously we can't do any customiztion before the command executes. This PR adds another @CommandCustomizers annotation support for
 test classes, which allows users to customize their commands before it executes.
@blindpirate blindpirate force-pushed the blindpirate/command-customizer branch from a54836f to 2a64083 Compare August 31, 2018 00:53
@eriwen
Copy link
Contributor

eriwen commented Aug 31, 2018

Hey Bo, could you please describe the end user needs here?

@blindpirate
Copy link
Author

@eriwen Sure, I explained a little above. Please let me know if I didn't make myself clear, thanks!

@blindpirate
Copy link
Author

Also, it seems that we don't have any CI configuration for this repository, do we?

@blindpirate
Copy link
Author

@eriwen Could you please take a look? We've been bitten by network issues in sample tests for a long time....

@eriwen eriwen self-assigned this Sep 5, 2018
@eriwen
Polish command customizer implementation
12a4174 
Adjust the Command Builder to live with the Command model object
and have a bit more fluent API.

Change command "Customizer" to "Modifier" to more clearly convey the purpose
of this construct.
eriwen
eriwen suggested changes Sep 5, 2018
View reviewed changes
Copy link
Contributor

@eriwen eriwen left a comment

Choose a reason for hiding this comment

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

There are a couple changes needed, but I will make them since I was late giving feedback.

sample-check/src/main/java/org/gradle/samples/test/runner/SamplesRunner.java Outdated
@@ -17,13 +17,17 @@

import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.StringUtils;
import org.gradle.api.Transformer;
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels weird to use the Gradle API for transforming here. Is there another way to do this without Gradle?

sample-check/src/main/java/org/gradle/samples/test/customizer/CommandBuilder.java Outdated

import java.util.List;

public class CommandBuilder {
Copy link
Contributor

Choose a reason for hiding this comment

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

We ought to make this a part of Command itself and not a side part of sample-check

sample-check/src/main/java/org/gradle/samples/test/customizer/CommandCustomizer.java Outdated
* Customize the command before the execution runs.
*/

public interface CommandCustomizer {
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer the term "Modifier" here

@eriwen
Copy link
Contributor

eriwen commented Sep 5, 2018

@blindpirate I adjusted this PR slightly and merged with a1e9ad6:

Allow Samples to be modified instead of just Commands. This allows folks to prepend/append commands to all samples in addition to modifying them all.
I also changed up the terminology and moved things around a bit. The integration tests still pass, so I imagine this'll work just fine for Gradle.

I've been trying to figure out why we get "too long path" errors on Windows in the Gradle build using Exemplar v0.5+. I'm working on fixing this problem but happy to get your eyes on it if you need the fix sooner.

Please reopen this PR or an issue if what I've provided doesn't actually do what you need for Gradle.

@eriwen eriwen closed this Sep 5, 2018
@wolfs wolfs mentioned this pull request Sep 18, 2018
Closed
@britter britter deleted the blindpirate/command-customizer branch August 11, 2021 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eriwen eriwen eriwen requested changes

Assignees

@eriwen eriwen

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@blindpirate @eriwen