-
Notifications
You must be signed in to change notification settings - Fork 53
Move to Maven/Tycho for projects and build infrastructure #1445
Comments
@cdietrich I seem to understand that we must use Java 17, because
|
i dont get this point
|
Sorry, I can't see the current configuration of Jenkins jobs due to the outage, and up to now, I thought we were building with Java 11 (that's what I did in GitHub Actions). Since in the other projects, m2e wasn't being used, I realized that only now. My mistake :) |
see Line 93 in aab51e0
Line 14 in aab51e0
=> diffferent tps run with different java versions |
nightly tiggered from xtext-eclipse https://github.com/eclipse/xtext-eclipse/blob/ef670c7566a58d3b75337660708d7c7d661e8e20/Jenkinsfile#L27 |
@cdietrich @szarnekow when running in GitHub Actions, this particular test fails on Windows (works on macOS and Linux):
@Test
def void testCompileSymlinkedResource() {
val tstResources = new File("./batch-compiler-data/test-resources/").toURI().normalize().getPath()
val wsRootFile = new File(tstResources, "symlink-test-ws/")
val wsRootPath = wsRootFile.getPath()
val linkToCreate = wsRootPath + "/plain-folder/linked-src"
if (!createSymLink(tstResources + "/linked-folder/linked-src/", linkToCreate)) {
System.err.println(
"Symlink creation is not possible - skip test. org.eclipse.xtend.core.tests.compiler.batch.TestBatchCompiler.testCompileSymlinkedResource()")
return
}
assertTrue("plain-folder/linked-src/ is a symlink", isSymlink(new File(wsRootFile, "plain-folder/linked-src/")))
assertTrue("plain-folder/src/ is not a symlink", !isSymlink(new File(wsRootFile, "plain-folder/src/")))
batchCompiler.writeTraceFiles = true
batchCompiler.sourcePath = wsRootPath + "/plain-folder/src" + File.pathSeparator + wsRootPath +
"/plain-folder/linked-src"
val customOutput = wsRootPath + "/plain-folder/target"
batchCompiler.outputPath = customOutput
assertTrue(batchCompiler.compile)
assertTrue(new File(wsRootPath + "/plain-folder/target/Test.txt").exists)
assertEquals(2, new File(customOutput).list[dir, name|name.endsWith(".java")].size)
assertEquals(2, new File(customOutput).list[dir, name|name.endsWith("._trace")].size)
} with this error
I guess that's expected because Windows does not support links. I disabled such a test on Windows: LorenzoBettini@7176e61 |
you are referring to TestBatchCompiler? i wonder why this works in gradle called from eclipse. or does it not work on tycho only? |
i see just does a syserr. do you see a test fail? |
It does fail on GitHub Actions Windows: https://pipelines.actions.githubusercontent.com/serviceHosts/5e72f0c0-eb6b-432b-aef4-14c3a40c344b/_apis/pipelines/1/runs/5/signedlogcontent/2?urlExpires=2023-01-24T13%3A57%3A30.6657990Z&urlSigningMethod=HMACV1&urlSignature=ssQLzpbzPrF1hnXxJ69PoqxQzTChvm6J4dT2Y7mqnx4%3D
So the check you're mentioning succeeds, but then the assertion fails... I'll che that on my Windows machine once I've set it up |
hm on my windows, ran from eclipse, it is green. |
which windows version is there on github actions? maybe this has symlinks now |
Yes I had renamed that test (to follow the conventions). This is the environment of GitHub Actions Windows: https://github.com/actions/runner-images/blob/main/images/win/Windows2022-Readme.md |
i dont have access to any windows server, just 10 |
On my Windows 10 machine, from Eclipse, if I run that test (after removing my check at the beginning) I also get
In fact, if I try to run the command from a terminal I get
but if try to run I suspect that on GitHub Actions they provide WSL and bash so it might be that def private boolean isSymlink(File file) {
var File canon
if (file.getParent() === null) {
canon = file
} else {
var File canonDir = file.getParentFile().getCanonicalFile()
canon = new File(canonDir, file.getName())
}
return !canon.getCanonicalFile().equals(canon.getAbsoluteFile())
} I'd avoid trying to understand what's going on on GitHub Actions on Windows. By the way, now that I've enabled xtend.ide.tests, they work perfectly on linux and macOS (on GitHub Actions), but on Windows a few of the ide tests are flaky. For example,
Another time, that test succeeds but other two IDE tests were failing because they could not find org.eclipse.xtext.xbase.lib. UI tests have always been a PITA on Windows on GitHub Actions, so I'd say I disable the build on Windows, WDYT? |
you can disable it for now. i also think there might be some more tests failing down the road. |
So I'm going to revert this commit as well LorenzoBettini@7176e61 OK? I hadn't noticed the check for symlink |
yes. or we need to look why it actually fails |
I pushed my branch to xtext-xtend and I had to manually trigger the Jenkins build specifying 'latest' and 'jdk17', if I understood correctly. |
Om Master there are profiles |
Yes I know about these profiles, and for the moment I'm only using "latest". My question was related to the Jenkins job: I must specify build parameters correctly, right? However, the build is green! :) I have to inspect the missing tests |
if you want to run with java 17 and latest by default you need to change Jenkinsfile to run with 17 by default or keep 11 and call with |
@cdietrich some findings about the differences in executed tests:
Maybe because it was deceived by the fact that they were inner classes of a suite class: @RunWith(Suite.class)
@SuiteClasses({
ReadWriteAccessTest.FieldReadTest.class,
ReadWriteAccessTest.VariableReadTest.class,
})
@SuppressWarnings("all")
public class ReadWriteAccessTest {
public static class FieldReadTest extends AbstractXtendTestCase {
... so it might have executed the two inner test cases first as part of the suite and then as single discovered test cases. |
have no idea about these tests @szarnekow do you? |
do we want to execute suites at all? Suite naming convention? |
What I meant is that all the tests executed in the past are also executed now; it's just that the counting was misleading. Running suites does not make much sense in the CI since all tests are executed anyway. Maven and Tycho should keep track of executed test cases, avoiding running the same test case twice (because it is referred to by a suite). At least, that is what I recall. From the results above, gradle also runs inner classes. |
=> rename the class on master, problem solved? |
maybe, but I don't see that as we problem once we know the numbers to make sure all the tests executed in the past are executed now. |
@cdietrich general question: in many test projects of most Git repositories, we have tons of warnings of discouraged access to our own provisional internal APIs. From time to time, I adjust "x-friend" accordingly in the MANIFEST. Should I go on like that, or can I configure the test projects to ignore "Discouraged access" warnings? |
i dont know. am still waiting for review feedback from @szarnekow . |
I cherry-picked your commit... let's see |
17:16:00 [ERROR] Internal error: java.lang.IllegalArgumentException: bundleLocation not found: /home/jenkins/.m2/repository/org/eclipse/orbit/bundles/com.google.guava/30.1.0-SNAPSHOT/com.google.guava-30.1.0-SNAPSHOT.jar -> [Help 1] |
Once again, problems with Jenkins parameters... it was running for the Tycho build:
While I wanted What happened is: For the Maven build it used the defaults because it's not parameterized, so Tycho 2.7.5. Instead, if I remember correctly, the older TP profile forces to use an older version of Tycho. As announced by Tycho, recent versions of Tycho (I guess 2.7.5) introduced changes in the downloaded artifacts that are not consumable by older versions of Tycho. So the Tycho build, using an older version of Tycho, cannot reuse the .m2 downloaded artifacts of the more recent version of Tycho. I restarted the build with hopefully correct parameters. |
It does run the before and after for the test. If I modify FlakyFailsFourTimes and add @Before
public void before() {
System.out.println("Flaky Before");
}
@After
public void after() {
System.out.println("Flaky After");
} I see this output
|
ReadWriteAccessTest: The grouping into a suite is not necessary. These could just be regular top level tests. |
About the symlinks on Windows / Linux: java.nio.file.Files.createSymbolicLink(Path, Path, FileAttribute<?>...) might be preferable compared to running ln |
@szarnekow thanks for the clarification on flaky! I also extracted the two tests and removed the suite. |
is there an easy way to test if
goes into the catch clause on windows server? update: it does not. => wonder what happens when you update the test hmmm: maybe i did something bad with c&p am still experimenting ok i added that class. |
another remark. you seem to use deprecated xvfb stuff. |
what do you mean "deprecated"? I'm using a GitHub Action that starts xvfb when running on Linux and doesn't when running on other OSes. This allows me to have a single GitHub Actions workflow. Of course, I could split the workflows. |
I see, thanks for the update. I'll update the workflow with an if condition then. |
This should fix it f6ef3e9 |
@cdietrich concerning Jenkinsfile and parameters, maybe I understand how to do that. First, the default options must be the first ones:
however, this changes the positions of the options, so if you had previously run a job using 'latest' when it was the last option now it would use the new last option 'r202212'. So, first, you have to adjust the parameters manually from the Jenkins job, and after running a job, everything should work concerning the defaults. |
yes the first parameter seems the first one, but i am not a jenkins file pro, so i cannot tell if it can be done better. |
Some trivia on build times for this project in the CI, only for the effective build and test stuff:
These numbers include TP and dependency resolution; in GitHub Actions, we use cache for dependencies. |
do we are faster as on master ?!? |
Nice nice! |
well on master there are two build stages, grandle and tycho, but most of the tests are still run through tycho on master as well, aren't they? Two stages might add some overhead, maybe. For sure, I think we can say we're not slower ;) I showed the numbers also to highlight how better GitHub Actions is in general ;) |
PR is ready with detailed notes #1446 |
with repo merge this is done |
See also eclipse/xtext-lib#499 and eclipse/xtext-core#2052 eclipse/xtext-extras#858
Development has started in my fork's branch:
https://github.com/LorenzoBettini/xtext-xtend/tree/lb_2052_maven_tycho
The text was updated successfully, but these errors were encountered: