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

chore: upgrade Spoon to 10.0.1-beta-2 #676

Merged
merged 6 commits into from
Dec 10, 2021
Merged

chore: upgrade Spoon to 10.0.1-beta-2 #676

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
20b8ab6
Upgrade spoon to 10.0.1-beta-2
algomaster99 Dec 9, 2021
f3b0280
Revert "fix: disable custom security manager for tests which indirect…
algomaster99 Dec 9, 2021
38e17db
Add test case for #600
algomaster99 Dec 10, 2021
e326cee
Add test case for #600
algomaster99 Dec 10, 2021
9992719
Add test case for #603
algomaster99 Dec 10, 2021
76c1e5b
Add test case for #602
algomaster99 Dec 10, 2021
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
14 changes: 1 addition & 13 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
<dependency>
<groupId>fr.inria.gforge.spoon</groupId>
<artifactId>spoon-core</artifactId>
<version>9.1.0-beta-11</version>
<version>10.0.1-beta-2</version>
<exclusions>
<exclusion>
<!-- must be excluded as it is signed, which causes problems with unsigned version from sonar -->
Expand Down Expand Up @@ -355,18 +355,6 @@
</plugins>
</build>
</profile>

<profile>
<id>spoonSnapshot</id>
<repositories>
<repository>
<id>ow2.org-snapshot</id>
<name>Maven Repository for Spoon Snapshots</name>
<url>https://repository.ow2.org/nexus/content/repositories/snapshots/</url>
<snapshots><enabled>true</enabled></snapshots>
</repository>
</repositories>
</profile>
Comment on lines -359 to -369
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 removed this XML config because it was not working for fetching SNAPSHOTs of Spoon when I was trying to debug Sorald using it. For using SNAPSHOT version, we can refer how it is done in diffmin because I know it works there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't want to do it like it is in diffmin, because that adds a repository to anything that depends on diffmin. In general, one should avoid adding repositories in pom files, here's a pretty good article about it.

Putting the repository in a profile means that it won't be included by default in dependent libraries/applications, which is desirable. As a concrete example of why it's bad to put repositories in pom files (without putting them in custom profiles): the old snapshot repository we used for Spoon was in its pom file, and it was pretty unreliable and went down every now and then. And every time it did, it wasn't possible to build Spoon or anything depending on Spoon, as that repository couldn't be reached. Even though nothing was fetched from it.

This works perfectly if you activate spoonSnapshot profile, e.g. mvn -P spoonSnapshot compile, but by default the repository isn't included (which is what we want).

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 did not know this. Thanks!

I was about to argue that it is still safe to enclose SNAPSHOT URLs in repositories however, sorald is also used as a dependency in some other projects, eg. repairnator. So I get your point. I will revert this change whenever we need the SNAPSHOT version next time.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I will revert this change whenever we need the SNAPSHOT version next time.

@algomaster99 Do you think we should open an issue not to forget this? Or should we do it right now?

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 don't think so opening an issue is necessary because it is not needed and whenever we want to work with the SNAPSHOT version again, we will know what we have to do. :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fine :)

</profiles>

<scm>
Expand Down
3 changes: 0 additions & 3 deletions src/test/java/sorald/ClasspathModeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ class ClasspathModeTest {
@Test
void resolveClasspathFrom_enablesRepairOfViolation_thatRequiresClasspathToDetect(
@TempDir File workdir) throws IOException {
// use the default security manager
System.setSecurityManager(null);

// arrange
org.apache.commons.io.FileUtils.copyDirectory(
TestHelper.PATH_TO_RESOURCES_FOLDER
Expand Down
6 changes: 0 additions & 6 deletions src/test/java/sorald/MavenLauncherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ public class MavenLauncherTest {
@Test
public void sorald_repairsProductionAndTestCode_inMavenProject(@TempDir File workdir)
throws IOException {
// use the default security manager
System.setSecurityManager(null);

// arrange
org.apache.commons.io.FileUtils.copyDirectory(
TestHelper.PATH_TO_RESOURCES_FOLDER
Expand Down Expand Up @@ -67,9 +64,6 @@ public void sorald_repairsProductionAndTestCode_inMavenProject(@TempDir File wor
@Test
void sorald_repairsRuleViolation_thatRequiresClasspathToDetect(@TempDir File workdir)
throws IOException {
// use the default security manager
System.setSecurityManager(null);

// arrange
org.apache.commons.io.FileUtils.copyDirectory(
TestHelper.PATH_TO_RESOURCES_FOLDER
Expand Down
3 changes: 0 additions & 3 deletions src/test/java/sorald/miner/WarningMinerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,6 @@ public void extractWarnings_statsOutput_containsExpectedAttributes() throws Exce
@Test
void canDetectRuleViolation_thatRequiresClasspath_whenResolvingClasspathInMavenProject(
@TempDir File tempdir) throws Exception {
// use the default security manager
System.setSecurityManager(null);

Path statsFile = tempdir.toPath().resolve("stats.json");
Path projectRoot =
TestHelper.PATH_TO_RESOURCES_FOLDER
Expand Down