-
Notifications
You must be signed in to change notification settings - Fork 334
Spring cleaning #3148
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
Spring cleaning #3148
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,17 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <parent> | ||
| <artifactId>apm-quartz-job-plugin</artifactId> | ||
| <artifactId>apm-quartz</artifactId> | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [for reviewer] using |
||
| <groupId>co.elastic.apm</groupId> | ||
| <version>1.38.1-SNAPSHOT</version> | ||
| </parent> | ||
| <modelVersion>4.0.0</modelVersion> | ||
|
|
||
| <artifactId>quartz-common</artifactId> | ||
| <artifactId>apm-quartz-common</artifactId> | ||
| <name>${project.groupId}:${project.artifactId}</name> | ||
|
|
||
| <properties> | ||
| <version.quartz>2.3.1</version.quartz> | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [for reviewer] the common part was compiled with quartz 2.x and contained some code that was only used with 2.x plugin, however making sure the common part is compiled with 1.x only is safer. |
||
| <version.quartz>1.7.3</version.quartz> | ||
| <apm-agent-parent.base.dir>${project.basedir}/../../..</apm-agent-parent.base.dir> | ||
| </properties> | ||
|
|
||
|
|
@@ -22,19 +22,6 @@ | |
| <version>${version.quartz}</version> | ||
| <scope>provided</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.springframework</groupId> | ||
| <artifactId>spring-context-support</artifactId> | ||
| <version>${version.spring}</version> | ||
| <scope>test</scope> | ||
| </dependency> | ||
|
|
||
| <dependency> | ||
| <groupId>org.quartz-scheduler</groupId> | ||
| <artifactId>quartz-jobs</artifactId> | ||
| <version>${version.quartz}</version> | ||
| <scope>test</scope> | ||
| </dependency> | ||
|
Comment on lines
-25
to
-37
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [for reviewer] those were only required for quartz 2.x |
||
| </dependencies> | ||
|
|
||
| <build> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,34 +21,25 @@ | |
| import co.elastic.apm.agent.AbstractInstrumentationTest; | ||
| import co.elastic.apm.agent.impl.ElasticApmTracer; | ||
| import co.elastic.apm.agent.impl.TracerInternalApiUtils; | ||
| import co.elastic.apm.agent.tracer.Outcome; | ||
| import co.elastic.apm.agent.impl.transaction.Span; | ||
| import co.elastic.apm.agent.impl.transaction.Transaction; | ||
| import co.elastic.apm.agent.tracer.Outcome; | ||
| import org.junit.jupiter.api.AfterEach; | ||
| import org.junit.jupiter.api.Assumptions; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.quartz.JobBuilder; | ||
| import org.quartz.JobDetail; | ||
| import org.quartz.JobExecutionException; | ||
| import org.quartz.Scheduler; | ||
| import org.quartz.SchedulerException; | ||
| import org.quartz.SimpleTrigger; | ||
| import org.quartz.impl.StdSchedulerFactory; | ||
| import org.quartz.jobs.DirectoryScanJob; | ||
| import org.quartz.jobs.DirectoryScanListener; | ||
|
|
||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
|
|
||
| abstract class AbstractJobTransactionNameInstrumentationTest extends AbstractInstrumentationTest { | ||
|
|
||
| private Scheduler scheduler; | ||
| protected Scheduler scheduler; | ||
|
|
||
| @BeforeEach | ||
| void prepare() throws SchedulerException { | ||
|
|
@@ -105,15 +96,7 @@ public void testAgentPaused() throws SchedulerException { | |
| assertThat(objectPoolFactory.getSpanPool().getRequestedObjectCount()).isEqualTo(spanCount); | ||
| } | ||
|
|
||
| @Test | ||
| void testSpringJob() throws SchedulerException { | ||
| Assumptions.assumeFalse(ignoreTestSpringJob()); | ||
|
|
||
| JobDetail job = buildJobDetailTestSpringJob("dummyJobName", "group1"); | ||
| scheduler.scheduleJob(job, createTrigger()); | ||
|
|
||
| verifyTransactionFromJobDetails(job, Outcome.SUCCESS); | ||
| } | ||
|
|
||
| @Test | ||
| void testJobWithResult() throws SchedulerException { | ||
|
|
@@ -124,24 +107,6 @@ void testJobWithResult() throws SchedulerException { | |
| assertThat(transaction.getResult()).isEqualTo("this is the result"); | ||
| } | ||
|
|
||
| @Test | ||
| void testDirectoryScan() throws SchedulerException, IOException { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [for reviewer] this was only relevant for quartz 2.x (hence the |
||
| Assumptions.assumeFalse(ignoreDirectoryScanTest()); | ||
|
|
||
| Path directoryScanTest = Files.createTempDirectory("DirectoryScanTest"); | ||
|
|
||
| final JobDetail job = JobBuilder.newJob(DirectoryScanJob.class) | ||
| .withIdentity("dummyJobName") | ||
| .usingJobData(DirectoryScanJob.DIRECTORY_NAME, directoryScanTest.toAbsolutePath().toString()) | ||
| .usingJobData(DirectoryScanJob.DIRECTORY_SCAN_LISTENER_NAME, TestDirectoryScanListener.class.getSimpleName()) | ||
| .build(); | ||
|
|
||
| scheduler.getContext().put(TestDirectoryScanListener.class.getSimpleName(), new TestDirectoryScanListener()); | ||
| scheduler.scheduleJob(job, createTrigger()); | ||
|
|
||
| verifyTransactionFromJobDetails(job, Outcome.SUCCESS); | ||
| } | ||
|
|
||
| @Test | ||
| void testJobWithException() throws SchedulerException { | ||
| JobDetail job = buildJobDetailTestJobWithException("dummyJobName"); | ||
|
|
@@ -174,16 +139,4 @@ public Transaction verifyTransaction(Transaction transaction, String expectedNam | |
|
|
||
| abstract JobDetail buildJobDetailTestJobWithException(String name); | ||
|
|
||
| abstract JobDetail buildJobDetailTestSpringJob(String name, String groupName); | ||
|
|
||
| abstract boolean ignoreTestSpringJob(); | ||
|
|
||
| abstract boolean ignoreDirectoryScanTest(); | ||
|
|
||
| public static class TestDirectoryScanListener implements DirectoryScanListener { | ||
|
|
||
| @Override | ||
| public void filesUpdatedOrAdded(File[] files) { | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,24 +1,25 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
| <packaging>pom</packaging> | ||
| <modules> | ||
| <module>quartz-common</module> | ||
| <module>apm-quartz-1-plugin</module> | ||
| <module>apm-quartz-2-plugin</module> | ||
| </modules> | ||
|
|
||
| <parent> | ||
| <artifactId>apm-agent-plugins</artifactId> | ||
| <groupId>co.elastic.apm</groupId> | ||
| <version>1.38.1-SNAPSHOT</version> | ||
| </parent> | ||
|
|
||
| <artifactId>apm-quartz-job-plugin</artifactId> | ||
| <artifactId>apm-quartz</artifactId> | ||
| <name>${project.groupId}:${project.artifactId}</name> | ||
| <packaging>pom</packaging> | ||
|
|
||
| <properties> | ||
| <apm-agent-parent.base.dir>${project.basedir}/../..</apm-agent-parent.base.dir> | ||
| </properties> | ||
|
|
||
| <modules> | ||
| <module>apm-quartz-common</module> | ||
| <module>apm-quartz-plugin-1</module> | ||
| <module>apm-quartz-plugin-2</module> | ||
| </modules> | ||
|
|
||
| </project> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -96,7 +96,7 @@ | |
| <groupId>org.springframework.boot</groupId> | ||
| <artifactId>spring-boot-maven-plugin</artifactId> | ||
| <!-- explicit version required as bom will not cover plugins, only dependencies --> | ||
| <version>${version.spring-boot}</version> | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [for reviewer] the global |
||
| <version>2.2.2.RELEASE</version> | ||
| <configuration> | ||
| <mainClass>co.elastic.apm.agent.springwebflux.testapp.WebFluxApplication</mainClass> | ||
| <!-- by default original jar is replaced, which makes impossible to reuse as dependency --> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[for reviewer] using same naming convention as grpc