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

#2934 Implemented tests for the classpath in tests #2935

Merged
merged 1 commit into from
Nov 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions embedded/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,32 @@
fun properties(key: String) = providers.gradleProperty(key)

dependencies {
// packaging, which modules to include into this one
listOf(
":plugin:core",
":lang.embedded:core",
).forEach {
compileOnly(project(it))
testCompileOnly(project(it))
testRuntimeOnly(project(it))
}
listOf(
":lang.embedded:core",
).forEach {
runtimeOnly(project(it))
intellijPlatform{
pluginModule(implementation(project(it)))
}
}

// additional compilation dependencies
listOf(
":plugin:core",
).forEach {
compileOnly(project(it))
testCompileOnly(project(it))
}

// Test dependencies
testImplementation(testFixtures(project(":plugin")))

// Plugin dependencies
intellijPlatform {
localPlugin(project(":plugin"))
}

// Useinstaller handling
intellijPlatform{
val platformVersionProvider: Provider<String> by rootProject.extra
create("IC", platformVersionProvider.get(), useInstaller = properties("useInstaller").get().toBoolean())
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2015-2023 Alexandr Evstigneev
* Copyright 2015-2024 Alexandr Evstigneev
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -29,15 +29,15 @@
@RunWith(Parameterized.class)
public class EmbeddedPerlPluginInstrumentationTest extends PerlInstrumentationTestCase {

public EmbeddedPerlPluginInstrumentationTest(@NotNull String ignoredName, @NotNull Class<?> cls) {
super(cls);
public EmbeddedPerlPluginInstrumentationTest(@NotNull String ignoredName, @NotNull Class<?> cls, @NotNull String patternString) {

Check warning

Code scanning / QDJVM

Unused declaration Warning test

Constructor is never used.
super(cls, patternString);
}

@Parameterized.Parameters(name = "{0}")
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][]{
{"plugin.core", PerlParserDefinition.class},
{"embedded", EmbeddedPerlParserDefinition.class}
{"plugin.core", PerlParserDefinition.class, PLUGIN_PATTERN_STRING},
{"embedded", EmbeddedPerlParserDefinition.class, EMBEDDED_PATTERN_STRING}
Comment on lines +39 to +40
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Constants are properly defined but EMBEDDED_PATTERN_STRING needs attention

The constants are defined in the parent class PerlInstrumentationTestCase, but there's a commented issue with EMBEDDED_PATTERN_STRING:

  • PLUGIN_PATTERN_STRING = "/plugin/build/libs/plugin-.+?\\.jar!"
  • EMBEDDED_PATTERN_STRING = "/embedded/core/build/libs/core-.+?\\.jar" (missing ! suffix and has incorrect path according to comment)

The comment states: "this is wrong, should be build/libs/lang\.embedded-.+?\.jar!"

🔗 Analysis chain

LGTM! Test data structure properly updated.

The test data array has been correctly extended to include the pattern strings.

Let's verify the pattern string constants are properly defined in the parent class:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that PLUGIN_PATTERN_STRING and EMBEDDED_PATTERN_STRING constants exist in PerlInstrumentationTestCase

# Search for the constant definitions
ast-grep --pattern 'class PerlInstrumentationTestCase {
  $$$
  $_ PLUGIN_PATTERN_STRING = $_;
  $$$
  $_ EMBEDDED_PATTERN_STRING = $_;
  $$$
}'

Length of output: 146


Script:

#!/bin/bash
# Let's try a different approach to find these constants
# First, let's find the parent test class file
fd -e java PerlInstrumentationTestCase

# Then let's search for the constants with ripgrep for better context
rg "PLUGIN_PATTERN_STRING|EMBEDDED_PATTERN_STRING" -A 2 -B 2

Length of output: 7561

});
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2015-2023 Alexandr Evstigneev
* Copyright 2015-2024 Alexandr Evstigneev
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -29,16 +29,16 @@

@RunWith(Parameterized.class)
public class HtmlMasonPluginInstrumentationTest extends PerlInstrumentationTestCase {
public HtmlMasonPluginInstrumentationTest(@NotNull String ignoredName, @NotNull Class<?> cls) {
super(cls);
public HtmlMasonPluginInstrumentationTest(@NotNull String ignoredName, @NotNull Class<?> cls, @NotNull String patternString) {

Check warning

Code scanning / QDJVM

Unused declaration Warning test

Constructor is never used.
super(cls, patternString);
}

@Parameterized.Parameters(name = "{0}")
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][]{
{"plugin.core", PerlParserDefinition.class},
{"mason.framework", MasonParserUtil.class},
{"htmlmason", HTMLMasonParserDefinition.class},
{"plugin.core", PerlParserDefinition.class, PLUGIN_PATTERN_STRING},
{"mason.framework", MasonParserUtil.class, MASON_FRAMEWORK_PATTERN_STRING},
{"htmlmason", HTMLMasonParserDefinition.class, MASON_PATTERN_STRING},
});
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2015-2023 Alexandr Evstigneev
* Copyright 2015-2024 Alexandr Evstigneev
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -29,16 +29,16 @@

@RunWith(Parameterized.class)
public class Mason2PluginInstrumentationTest extends PerlInstrumentationTestCase {
public Mason2PluginInstrumentationTest(@NotNull String ignoredName, @NotNull Class<?> cls) {
super(cls);
public Mason2PluginInstrumentationTest(@NotNull String ignoredName, @NotNull Class<?> cls, @NotNull String patternString) {

Check warning

Code scanning / QDJVM

Unused declaration Warning test

Constructor is never used.
super(cls, patternString);
}

@Parameterized.Parameters(name = "{0}")
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][]{
{"plugin.core", PerlParserDefinition.class},
{"mason.framework", MasonParserUtil.class},
{"mason2", Mason2ParserDefinition.class},
{"plugin.core", PerlParserDefinition.class, PLUGIN_PATTERN_STRING},
{"mason.framework", MasonParserUtil.class, MASON_FRAMEWORK_PATTERN_STRING},
{"mason2", Mason2ParserDefinition.class, MASON2_PATTERN_STRING},
});
}
}
10 changes: 5 additions & 5 deletions mojo/src/test/java/unit/MojoPluginInstrumentationTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2015-2023 Alexandr Evstigneev
* Copyright 2015-2024 Alexandr Evstigneev
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -29,15 +29,15 @@
@RunWith(Parameterized.class)
public class MojoPluginInstrumentationTest extends PerlInstrumentationTestCase {

public MojoPluginInstrumentationTest(@NotNull String ignoredName, @NotNull Class<?> cls) {
super(cls);
public MojoPluginInstrumentationTest(@NotNull String ignoredName, @NotNull Class<?> cls, @NotNull String patternString) {

Check warning

Code scanning / QDJVM

Unused declaration Warning test

Constructor is never used.
super(cls, patternString);
}

@Parameterized.Parameters(name = "{0}")
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][]{
{"plugin.core", PerlParserDefinition.class},
{"mojo.core", MojoliciousParserDefinition.class},
{"plugin.core", PerlParserDefinition.class, PLUGIN_PATTERN_STRING},
{"mojo.core", MojoliciousParserDefinition.class, MOJO_PATTERN_STRING},
});
}
}
46 changes: 23 additions & 23 deletions plugin/src/test/java/unit/PerlPluginInstrumentationTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2015-2023 Alexandr Evstigneev
* Copyright 2015-2024 Alexandr Evstigneev
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -47,33 +47,33 @@
@RunWith(Parameterized.class)
public class PerlPluginInstrumentationTest extends PerlInstrumentationTestCase {

public PerlPluginInstrumentationTest(@NotNull String ignoredName, @NotNull Class<?> cls) {
super(cls);
public PerlPluginInstrumentationTest(@NotNull String ignoredName, @NotNull Class<?> cls, @NotNull String patternString) {

Check warning

Code scanning / QDJVM

Unused declaration Warning test

Constructor is never used.
super(cls, patternString);
}

@Parameterized.Parameters(name = "{0}")
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][]{
{"asdf", AsdfTestUtil.getInstrumentationTestClass()},
{"berrybrew", BerryBrewTestUtil.getInstrumentationTestClass()},
{"carton", PerlCartonDirectoryConfigurationProvider.class},
{"copyright", PerlCopyrightsVariablesProvider.class},
{"core", PerlParserDefinition.class},
{"coverage", PerlCoverageEnabledConfiguration.class},
{"cpan", PerlInstallPackagesWithCpanAction.class},
{"cpanminus", PerlInstallCpanmAction.class},
{"debugger", PerlDebuggerProgramRunner.class},
{"docker", PerlDockerTestUtil.getInstrumentationTestClass()},
{"idea", PerlIdeaTestUtil.getInstrumentationTestClass()},
{"intelliLang", PerlInjectionSupport.class},
{"makeMaker", PerlMakeMakerDirectoryConfigurationProvider.class},
{"moduleBuild", PerlModuleBuildDirectoryConfigurationProvider.class},
{"perlInstall", PerlInstallHandlerBase.class},
{"perlbrew", PerlBrewTestUtil.getInstrumentationTestClass()},
{"plenv", PlenvTestUtil.getInstrumentationTestClass()},
{"profiler", PerlProfilerBundle.class},
{"terminal", PerlLocalTerminalCustomizer.class},
{"wsl", PerlWslInputFilterProvider.class},
{"asdf", AsdfTestUtil.getInstrumentationTestClass(), PLUGIN_PATTERN_STRING},
{"berrybrew", BerryBrewTestUtil.getInstrumentationTestClass(), PLUGIN_PATTERN_STRING},
{"carton", PerlCartonDirectoryConfigurationProvider.class, PLUGIN_PATTERN_STRING},
{"copyright", PerlCopyrightsVariablesProvider.class, PLUGIN_PATTERN_STRING},
{"core", PerlParserDefinition.class, PLUGIN_PATTERN_STRING},
{"coverage", PerlCoverageEnabledConfiguration.class, PLUGIN_PATTERN_STRING},
{"cpan", PerlInstallPackagesWithCpanAction.class, PLUGIN_PATTERN_STRING},
{"cpanminus", PerlInstallCpanmAction.class, PLUGIN_PATTERN_STRING},
{"debugger", PerlDebuggerProgramRunner.class, PLUGIN_PATTERN_STRING},
{"docker", PerlDockerTestUtil.getInstrumentationTestClass(), PLUGIN_PATTERN_STRING},
{"idea", PerlIdeaTestUtil.getInstrumentationTestClass(), PLUGIN_PATTERN_STRING},
{"intelliLang", PerlInjectionSupport.class, PLUGIN_PATTERN_STRING},
{"makeMaker", PerlMakeMakerDirectoryConfigurationProvider.class, PLUGIN_PATTERN_STRING},
{"moduleBuild", PerlModuleBuildDirectoryConfigurationProvider.class, PLUGIN_PATTERN_STRING},
{"perlInstall", PerlInstallHandlerBase.class, PLUGIN_PATTERN_STRING},
{"perlbrew", PerlBrewTestUtil.getInstrumentationTestClass(), PLUGIN_PATTERN_STRING},
{"plenv", PlenvTestUtil.getInstrumentationTestClass(), PLUGIN_PATTERN_STRING},
{"profiler", PerlProfilerBundle.class, PLUGIN_PATTERN_STRING},
{"terminal", PerlLocalTerminalCustomizer.class, PLUGIN_PATTERN_STRING},
{"wsl", PerlWslInputFilterProvider.class, PLUGIN_PATTERN_STRING},
});
}
}
29 changes: 25 additions & 4 deletions plugin/src/testFixtures/java/base/PerlInstrumentationTestCase.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2015-2023 Alexandr Evstigneev
* Copyright 2015-2024 Alexandr Evstigneev
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -23,19 +23,31 @@
import org.junit.Test;
import org.junit.experimental.categories.Category;

import java.util.regex.Pattern;
import java.util.stream.Stream;

@SuppressWarnings("JUnitMixedFramework")
@Category(Light.class)
public abstract class PerlInstrumentationTestCase extends BasePlatformTestCase {

private final Class<?> myClass;
protected static final String PLUGIN_PATTERN_STRING = "/plugin/build/libs/plugin-.+?\\.jar!";
// this is wrong, should be build/libs/lang\\.embedded-.+?\\.jar!
protected static final String EMBEDDED_PATTERN_STRING = "/embedded/core/build/libs/core-.+?\\.jar";
protected static final String MOJO_PATTERN_STRING = "/mojo/core/build/libs/core-.+?\\.jar!";
protected static final String TT2_PATTERN_STRING = "/tt2/core/build/libs/core-.+?\\.jar!";
protected static final String MASON_FRAMEWORK_PATTERN_STRING = "/mason/framework/build/libs/lang\\.mason\\.framework-.+?\\.jar!";
protected static final String MASON_PATTERN_STRING = "/mason/htmlmason/core/build/libs/core-.+?\\.jar!";
protected static final String MASON2_PATTERN_STRING = "/mason/mason2/core/build/libs/core-.+?\\.jar!";
Comment on lines +33 to +40
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect pattern for EMBEDDED_PATTERN_STRING

As noted in the comment, the pattern for embedded library path is incorrect. It's missing the '!' terminator and uses incorrect path structure.

Apply this fix:

-  protected static final String EMBEDDED_PATTERN_STRING = "/embedded/core/build/libs/core-.+?\\.jar";
+  protected static final String EMBEDDED_PATTERN_STRING = "/embedded/build/libs/lang\\.embedded-.+?\\.jar!";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
protected static final String PLUGIN_PATTERN_STRING = "/plugin/build/libs/plugin-.+?\\.jar!";
// this is wrong, should be build/libs/lang\\.embedded-.+?\\.jar!
protected static final String EMBEDDED_PATTERN_STRING = "/embedded/core/build/libs/core-.+?\\.jar";
protected static final String MOJO_PATTERN_STRING = "/mojo/core/build/libs/core-.+?\\.jar!";
protected static final String TT2_PATTERN_STRING = "/tt2/core/build/libs/core-.+?\\.jar!";
protected static final String MASON_FRAMEWORK_PATTERN_STRING = "/mason/framework/build/libs/lang\\.mason\\.framework-.+?\\.jar!";
protected static final String MASON_PATTERN_STRING = "/mason/htmlmason/core/build/libs/core-.+?\\.jar!";
protected static final String MASON2_PATTERN_STRING = "/mason/mason2/core/build/libs/core-.+?\\.jar!";
protected static final String PLUGIN_PATTERN_STRING = "/plugin/build/libs/plugin-.+?\\.jar!";
// this is wrong, should be build/libs/lang\\.embedded-.+?\\.jar!
protected static final String EMBEDDED_PATTERN_STRING = "/embedded/build/libs/lang\\.embedded-.+?\\.jar!";
protected static final String MOJO_PATTERN_STRING = "/mojo/core/build/libs/core-.+?\\.jar!";
protected static final String TT2_PATTERN_STRING = "/tt2/core/build/libs/core-.+?\\.jar!";
protected static final String MASON_FRAMEWORK_PATTERN_STRING = "/mason/framework/build/libs/lang\\.mason\\.framework-.+?\\.jar!";
protected static final String MASON_PATTERN_STRING = "/mason/htmlmason/core/build/libs/core-.+?\\.jar!";
protected static final String MASON2_PATTERN_STRING = "/mason/mason2/core/build/libs/core-.+?\\.jar!";


private final @NotNull Class<?> myClass;

protected PerlInstrumentationTestCase(@NotNull Class<?> aClass) {
private final @NotNull Pattern myClassPathPattern;

protected PerlInstrumentationTestCase(@NotNull Class<?> aClass, @NotNull String patternString) {
myClass = aClass;
myClassPathPattern = Pattern.compile(patternString);
}

@SuppressWarnings("JUnitMixedFramework")
@Test
public void testDependencyInstrumentation() {
var classLoader = myClass.getClassLoader();
Expand All @@ -45,4 +57,13 @@ public void testDependencyInstrumentation() {
"Class " + myClass + " does not look instrumented, classpaths: " + pathClassLoader.getClassPath().getBaseUrls(),
Stream.of(myClass.getDeclaredMethods()).anyMatch(m -> m.getName().startsWith("$$$reportNull$$$")));
}

@Test
public void testProperClassSource() {
var className = myClass.getName();
var classPath = myClass.getResource("/" + className.replace('.', '/') + ".class").getPath();
if (!myClassPathPattern.matcher(classPath).find()) {
fail("Classpath for the " + className + " is expected to match " + myClassPathPattern + ", but was " + classPath);
}
}
Comment on lines +61 to +68
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider improving test robustness and error reporting

The new test method is good, but could be enhanced in two ways:

  1. The error message could include an example of what the pattern would match
  2. The path comparison might fail on Windows due to different path separators

Consider this enhancement:

   @Test
   public void testProperClassSource() {
     var className = myClass.getName();
-    var classPath = myClass.getResource("/" + className.replace('.', '/') + ".class").getPath();
+    var classPath = myClass.getResource("/" + className.replace('.', '/') + ".class")
+      .getPath().replace('\\', '/');  // Normalize path separators
     if (!myClassPathPattern.matcher(classPath).find()) {
-      fail("Classpath for the " + className + " is expected to match " + myClassPathPattern + ", but was " + classPath);
+      fail(String.format("Classpath for %s is invalid.%nExpected pattern: %s%nActual path: %s%nExample valid path: %s",
+                        className, myClassPathPattern, classPath,
+                        "/plugin/build/libs/plugin-1.0.0.jar!"));
     }
   }

Committable suggestion skipped: line range outside the PR's diff.

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2015-2023 Alexandr Evstigneev
* Copyright 2015-2024 Alexandr Evstigneev
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -29,15 +29,15 @@
@RunWith(Parameterized.class)
public class TemplateToolkitPluginInstrumentationTest extends PerlInstrumentationTestCase {

public TemplateToolkitPluginInstrumentationTest(@NotNull String ignoredName, @NotNull Class<?> cls) {
super(cls);
public TemplateToolkitPluginInstrumentationTest(@NotNull String ignoredName, @NotNull Class<?> cls, @NotNull String patternString) {

Check warning

Code scanning / QDJVM

Unused declaration Warning test

Constructor is never used.
super(cls, patternString);
}

@Parameterized.Parameters(name = "{0}")
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][]{
{"plugin.core", PerlParserDefinition.class},
{"tt2", TemplateToolkitParserDefinition.class}
{"plugin.core", PerlParserDefinition.class, PLUGIN_PATTERN_STRING},
{"tt2", TemplateToolkitParserDefinition.class, TT2_PATTERN_STRING}
});
}
}
Loading