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

Use 2023-09 simrel #2848

Merged
merged 4 commits into from
Sep 28, 2023
Merged

Use 2023-09 simrel #2848

merged 4 commits into from
Sep 28, 2023

Conversation

akurtakov
Copy link
Member

As default repo in plugins that have such and in tests.

@github-actions
Copy link

github-actions bot commented Sep 23, 2023

Test Results

   561 files  ±0     561 suites  ±0   4h 11m 13s ⏱️ - 24m 57s
   363 tests ±0     356 ✔️  - 1    6 💤 ±0  0 ±0  1 🔥 +1 
1 089 runs  ±0  1 067 ✔️  - 3  19 💤 ±0  0 ±0  3 🔥 +3 

For more details on these errors, see this check.

Results for commit a5c1f3c. ± Comparison against base commit 03e790c.

♻️ This comment has been updated with latest results.

@laeubi
Copy link
Member

laeubi commented Sep 23, 2023

@akurtakov what do you think about moving this to TychoConstants as it is also used in several places in Tycho (no test) code?

@@ -36,7 +36,7 @@ public class EnvironmentUtil {

private static final String MAVEN_HOME_INFO = "Maven home:";

public static final String ECLIPSE_LATEST = "https:////download.eclipse.org/releases/2022-12/";
public static final String ECLIPSE_LATEST = "https:////download.eclipse.org/releases/2023-09/";
Copy link
Member

Choose a reason for hiding this comment

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

do you know hy we have the four times / here? it seems to not harm but looks strange...

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 would guess 2 // for the protocol and 2 more to escape them somewhere in the code.

Copy link
Member

Choose a reason for hiding this comment

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

This looks suspicious... if we need escaping it should not be done in the constant itself ...

@akurtakov
Copy link
Member Author

@laeubi Is there a chance you can help with the last failing test? I'm kind of lost on this one.

@laeubi
Copy link
Member

laeubi commented Sep 27, 2023

I'll try to take a look tomorrow! This test is a bit special because it uses not the junit bundles from the target but the maven ones, as the test complains about missing junit maybe we need to explicitly include junit somehow explicitly, from the changes made this might previously be pulled in by hamcrest or something.

@laeubi
Copy link
Member

laeubi commented Sep 27, 2023

By the way if I wanted to debug this I probably would enable the display of the bundles for this test project and see if anything looks suspicious (e.g. juni4 really missing or pointing not to the local maven repository).

@akurtakov
Copy link
Member Author

The test itself is JUnit 5 so it should work just fine without junit.jar (aka 4.x), no?

@laeubi
Copy link
Member

laeubi commented Sep 27, 2023

The test complains that the vintage engine requires junit4 ... if I rember correctly JDT defines JUNIT5 = JUNIT5 Vintage... maybe it would be great to have a pure JUNIT5 Container without any vintage/junit4...

@akurtakov
Copy link
Member Author

I've tried by removing vintage engine from the junit5 bundles and the test works fine so what happens is that vintage engine fails and that way it even blocks other engines from looking up for tests.

@laeubi
Copy link
Member

laeubi commented Sep 28, 2023

I have now created

So we need to somehow convince Tycho to include junit4 when vintage engine is included, I'm just a bit confused why it is not select anymore automatically...

@laeubi
Copy link
Member

laeubi commented Sep 28, 2023

I now extracted the easy part here:

@akurtakov
Copy link
Member Author

akurtakov commented Sep 28, 2023

@laeubi Do you agree with using

<configuration>
  <excludeJUnit5Engines>
    <engine>junit-vintage</engine>
  </excludeJUnit5Engines>
</configuration>

to workaround the issue for now?

@laeubi
Copy link
Member

laeubi commented Sep 28, 2023

I have now tested it and at first everything seems okay but then surefire fails, I can fix the test by adding

<dependency>
		  <groupId>org.apache.servicemix.bundles</groupId>
		  <artifactId>org.apache.servicemix.bundles.junit</artifactId>
		  <version>4.13.2_1</version>
		  <scope>test</scope>
</dependency>

as an explicit dependency ... so it seems that something goes wrong maybe when injecting the dependencies even though I can't see how/why this happens need to debug a bit further.

@laeubi
Copy link
Member

laeubi commented Sep 28, 2023

The junit dependency is currently injected as

 <dependency>
      <groupId>p2.eclipse.plugin</groupId>
      <artifactId>org.apache.servicemix.bundles.junit</artifactId>
      <version>4.13.2.1</version>
      <type>eclipse-plugin</type>
      <scope>system</scope>
      <systemPath>/tmp/file not yet available5847474459691235710.tmp</systemPath>
    </dependency>

so for some reason p2 things that the file is not (yet) there and then surefire can not use it of course ... will try to provide a fix for that...

@laeubi
Copy link
Member

laeubi commented Sep 28, 2023

This should fix the issue we see here:

As default repo in tests.
Bundle id changes.
Simplified setup a bit while at it to be more like rest of the test
suite.
Add JavaSE-17 osgi.ee capability to allow resolving.
Simplify test setup while at it.
@akurtakov
Copy link
Member Author

Jenkins succeeded. Merging.

@akurtakov akurtakov merged commit 05367ee into eclipse-tycho:master Sep 28, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants