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

FIX CI - Issue #1253 #1256

Merged
merged 105 commits into from
May 11, 2023

Conversation

sofiabobadilla
Copy link
Contributor

Change GitHub.connectAnonymously() in PatchFilterTest.java to connect with Github using Jenkins environment variables instead.

Copy link

@algomaster99 algomaster99 left a comment

Choose a reason for hiding this comment

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

I don't have suggestions specific to the project, but I thought I would write two cents about commit messages.

Make sure that a commit message explains what exactly it does. The first two commit messages are 'PatchFilterTest connects to Github with Jenkins field'. There are two problems with this:

  1. I, personally, do not immediately understand what it is supposed to do.
    1. Proposed solution: A better commit message, in my opinion, would be 'use Jenkins credentials to connect to GitHub'
  2. The commit message is same which means they both do the same thing.
    1. It is usually nice to split commits, but not when they have the same purpose. One commit should do one task. It is arbitrary, however, for this case, it seems that you are doing same things for multiple invocations consolidating them into one single task. Such independent commits make reversal easy.

@monperrus
Copy link
Contributor

Great, CI seems to fail because of #1254 (comment)

@monperrus
Copy link
Contributor

CI still failing. @javierron does Jenkins already know about credential GITHUB_OAUTH?

@javierron
Copy link
Contributor

javierron commented Sep 2, 2022

@monperrus @Sofi1410

Credentials used in the test class seem to be fine.

The problem is that the calls from inside the tested class don't use the GitHub client library and are probably being rate-limited (unclear from logs). We did it that way to be able "to change target URLs to a mirror in case rate limits are exceeded".

In case rate-limits are actually the issue, we would need to either use mirrors, or the GitHub client w/ token.

@monperrus
Copy link
Contributor

monperrus commented Sep 3, 2022 via email

@sofiabobadilla
Copy link
Contributor Author

The CI keeps falling with this error:
Caused by: java.lang.ClassNotFoundException: org.codehaus.plexus.util.xml.Xpp3DomBuilder$InputLocationBuilder

Apparently, this is a common bug related to dependencies. I will try this solution :https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=948286

@monperrus
Copy link
Contributor

monperrus commented Sep 23, 2022 via email

…spirals/repairnator/pipeline/TestPipeline.java & TestPipelineFaultLocalization.java & TestPipelinebTravisMode.java to avoid RunTImeProblems in the CI, all test passed in execution 15 PR-1256
@sofiabobadilla
Copy link
Contributor Author

At the same time I'm not entirely sure if it is related to the error I comment above, but, on local, the test fr.inria.spirals.repairnator.process.inspectors.TestProjectInspector4Maven.testPatchFailingProject reproduces the same error as CI only if I don't provide the M2_HOME var, but this parameter is present o the Jenkins File so I still don't understand completely what is going on.

@algomaster99
Copy link

Could you share the experiments you did last time when you checked the bug?

I did my experiment here. Looking back at it, I did not do anything because the problem did not persist.

So far the error keps being:

Could not download Sonar Java plugin
-Caused by: java.nio.file.NoSuchFileException: /home/jenkins/.cache/sorald/sonar-java-plugin.jar

I double check the URL in sorald (https://github.com/ASSERT-KTH/sorald/blob/380e8158e56dc5464f9e750231b47bbc43b9d94a/sorald/src/main/resources/config.properties#L1) and it is working.

Successfully reaching this URL is flaky. Sometimes, it is fetched successfully, sometimes the CI just fails. I would recommend you to run you CI job again and it should pass.

@sofiabobadilla
Copy link
Contributor Author

Could you share the experiments you did last time when you checked the bug?

I did my experiment here. Looking back at it, I did not do anything because the problem did not persist.

I'm looking at the execution: https://ci.eclipse.org/repairnator/job/repairnator-ci/job/PR-1263/2/testReport/
And there is no info related to sorald, the test report doesn't shows result from TestPipelineSoraldRepairTool. From my POV I don't see how the problem is solved.

Successfully reaching this URL is flaky. Sometimes, it is fetched successfully, sometimes the CI just fails. I would recommend you to run you CI job again and it should pass.

I don't understand how reaching the URL could be flaky for this, could you extend a bit on this?

@algomaster99
Copy link

Now, when I look back at the report, it seems that the test pipeline timed out and it never really executed the Sorald tests. I was trying to minimise the tests run in the build and accidentally prevented execution of the Sorald tests. I cannot test it right now because I am away from keyboard.

However, the errors which you are posting it is because of failure to fetch the jar at runtime.

I don't understand how reaching the URL could be flaky for this, could you extend a bit on this?

It is not flaky for this. It is flaky in general. Sometimes the CI is just not able to reach the URL due to network issues. It happens in Sorald's master branch as well, but it is quite rare (1 in 20 builds).
The best thing to do here is to manually run it again. I also do it and 99% of the times the problem disappears if it could not fetch the jar in the previous build. Or you could have a check in the CI to run Sorald's test at least 2-3 times so that at least once you fetch the Jar.

@sofiabobadilla
Copy link
Contributor Author

Related to last commit: https://bugs.eclipse.org/bugs/show_bug.cgi?id=575400

@sofiabobadilla sofiabobadilla marked this pull request as ready for review May 10, 2023 15:05
@monperrus monperrus merged commit 14bcf67 into eclipse-repairnator:master May 11, 2023
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.

6 participants