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

WIP: Dockerfile for Astor 1.1 #312

Closed
wants to merge 16 commits into from

Conversation

lapplislazuli
Copy link

Good Afternoon @martinezmatias ,

this is the work in progress of adding a dockerfile, however I still run into some issues that would need a second pair of eyes and are at least easily reproducible.

Changes

The current changes are:

  • Adding Mylin as extra repository in Pom
  • Minor Changes in looking for Java (as Java Paths are different in my installations)
  • Adding tdurieux's plugin to the libraries in have a manual install
  • Dockerfile hardcoded for Math-Issue-280 and an simple entrypoint
  • Adding Java 7 as a dependency for the Dockerfile
  • Add a log4j2.xml that is used in the container

To build run

docker build . -t astor:unstable

Which takes ~5 minutes on my mac.

Occurring Errors

When running with any mode of gzoltar, it fails as gzoltar does not find suspicious code.
This behaviour is identical on my machine outside of the container and occurs for any bug I checked so far.
To produce the error, run

docker run --env astor_faultlocalization=gzoltar astor:unstable

My supervisor ran the current version on Java8 and while failing somewhere else, gzoltar found suspicious pieces.
Could it be that gzoltar does not function (correctly) above java 8?

When running with CoCoSpoon, the process starts but terminates after 100 generations without finding a solution.
What's weird about this is that I set the generations to 3000.

To produce this error, run

docker run --env astor_mode=jMutRepair --env astor_faultlocalization=CoCoSpoon astor:unstable

I attached the logs from the runs above.
jGenProg_math280.log
jkali_math280.log
jMutRepair_math280.log

Aimed Solution

On a sidenote while working with everything, in general I would like to have 3 Docker images

  1. Astor:Latest (Java 11)
  2. Astor:Latest-Debug (Java 11 with debug compilation)
  3. Astor:Legacy (including Java 7)

Then experiments and getting started's could be easily described as docker-compose-files.

@lapplislazuli lapplislazuli marked this pull request as draft April 9, 2021 11:51
@lapplislazuli
Copy link
Author

When changing maven to use jdk8, everything seems to work as expected.
Atleast Gzoltar finds suspicious code as expected in j8, and does not do so in j11.

@martinezmatias
Copy link
Collaborator

martinezmatias commented Apr 13, 2021

Hi @lapplislazuli

Atleast Gzoltar finds suspicious code as expected in j8, and does not do so in j11.

Yes. We have seen that GZoltar works fine with j8 (in our previous experiments the problem of j8 came from Defects4J) but does not work in j11

@lapplislazuli
Copy link
Author

Good Afternoon @martinezmatias ,

Astor now terminates for Math280 (without finding a solution) but at least all Gzoltar Issues seem to be ironed out.
At the moment I added the new Gzoltar-Libraries as new dependencies and point to them with a path,
invoking the console tools from java and then reading in the .csv files similar to what was present in GzoltarClientMaster (just that the .csv's look a little different, but same principle).

There are three points where I could need feedback:

  • There are a few TODOs in the GzoltarFaultLocalization on what attributes need to be filled and which not, where a short comment would be great
  • The gzoltarpath currently comes from the astor.properties, but I have not figured out how to overwrite it on command line, which makes either the IDE run or the Docker run clunky
  • A short feedback whether the rest of the program seems to run normal for you, as ... I have no point of comparison

After your feedback I'd de-hardcode the docker-part and try to run some of the examples with docker compose.

Best
Leonhard

@byteSamurai
Copy link

Would also close #132 if I am not mistaken. Pretty cool what you are doing here @lapplislazuli

@martinezmatias
Copy link
Collaborator

Hi @lapplislazuli

but I have not figured out how to overwrite it on command line, which makes either the IDE run or the Docker run clunky

It is possible: in the command line you can use the existing parameter -properties X:Y where X corresponds to the key of the property (in your case gzoltarpath) and Y the value /apps/astor/lib/moderngzoltar. You can overwrite several values in the same command -properties X:Y:W:Z where X and W are the keys, Y and Z the values.

At the moment I added the new Gzoltar-Libraries as new dependencies and point to them with a path

I did not understand when you say "added the new Gzoltar-Libraries as new dependencies". As far I understood, you removed the Gzoltar 1.X from the Pom.xml, and retrieves the path from the configuration file.
Is my interpretation correct? (I would say "Yes", as then you wrote "The gzoltarpath currently comes from the astor.properties,", but to be sure)

The gzoltarpath currently comes from the astor.properties,

I see the strategy and I would say that It works. However, I wonder what will happen if a user want to create a jar with all the dependencies (i.e. we usually create it for deploying on our experiments infractucture): in that case, they fat jar will not contain the classes from Gzoltar (as the dependency is not any more in the pom) but, at the same time, the path to Gzoltar from the configuration file will probably not exist. Do you see my point?

I am not sure about removing dependency to Gzoltar 0.X.

We have defined an extension point FaultLocalizationStrategy so we can have several FL implementation, actually we have currently 3 implemented: GZoltar 0.7 -default-, CoCoSpoon, and GZoltar 1.6.
Consequently, for interacting with the GZoltar 1.7 we should create a new extension of that point (i.e., a class that implement FaultLocalizationStrategy). Then, It would be simple to determine which strategy is used by default.
To sum up this point: we should keep dependency to 0.7 (at least we are use that GZ 1.7 is well tested), and the changes on GZoltarFaultLocalization.java should be done in a new class that implements FaultLocalizationStrategy.
WDYT?

Thanks a lot for the great progress on the issue!
Regards
Matias

@lapplislazuli
Copy link
Author

lapplislazuli commented Apr 29, 2021

@martinezmatias I am a bit stuck again 🤷‍♂️

In older Gzoltar there seems to be information on the file under inspection, in newer there is not. The best I have is package + class name, but that is not enough to match the CtClass Elements later, which gives the following errors:
image

I could also not spot any options in formatters on gzoltar side.

Here is the full log of the run
NoCtClass.log but it just prints the same error for all 8 variants for any suspicious line of code.
It then has no modificationpoints and stops trying after a 100 attempts.

Do you think we can go with a heuristic CtClass-match that does not need the file?

edit: OK I used the fully qualified classname (package+class) and now it's doing something, just the ctClasses for the Tests are missing but I want to filter them out anyway.

In terms of where to put gzoltar.jar's I also see issues with building the astor-with-dependencies. Gzoltar would have to be treated similar to resources I guess.
I also think it's better to have it in the dependencies, so it's clear that you need it, but then how would the jars go from the maven repository to the resources as the Gzoltar things should preferebly used as CLI.
Another option I see that would make the repository smaller is to provide a install.sh that downloads the jars. That one might not work on all dependencies (some are not available via maven/github) and also might not work on every OS.

@martinezmatias
Copy link
Collaborator

Hi @lapplislazuli

The error you point is when Astor cannot find the location of the suspicious. I suspect it happens when the suspicious is located inside a) anonymous class, b) inner class.

I also think it's better to have it in the dependencies, so it's clear that you need it, but then how would the jars go from the maven repository to the resources as the Gzoltar things should preferebly used as CLI.

When we run Astor in Eclipse is simple: if Gzoltar is a dependency (declared on the pom.xml) then it will be included in the classpath of Astor. In other words, if inside of Astor we call to System.getProperty("java.class.path"); we have as results all dependencies, including GZoltar. However, when we run Astor from command line having a single jar with dependencies, that solution does not work. One ugly option to call GZoltar from Astor as command line could be to pass the same classpath that Astor reveices, which would include GZoltar. (I say ugly because the classpath would have lot of classes not used by gzoltar)

Another option I see that would make the repository smaller is to provide a install.sh that downloads the jars. That one might not work on all dependencies (some are not available via maven/github) and also might not work on every OS.

It could be an option, however, It adds some complexity on the execution process.

Regards
Matias

… running, added a filter on test methods for suspicious code
@lapplislazuli
Copy link
Author

Goude Middag @martinezmatias,
The gzoltar part seems now pretty stable and happy.

The new patients are LauncherJunitProcess and the JunitNoLogExecutor.
Without any modification, it seems that the NoLog Executor is not properly started. The LauncherJunitProcess then tries to read in stdin but only finds the timezone and no data.

12:11:09.286 [main] ERROR main - Error reading the validation process
 output: 
America/Los_Angeles

Where the last line is an actual blank line.

If I try to start the process in a similar fashion:

ProcessBuilder pb2 = new ProcessBuilder(command)
	.directory(new File("/tmp/astor"))
	.redirectError(new File("/tmp/astor/JunitLauncherError.log"))
	.redirectOutput(new File("/tmp/astor/JunitLauncherOutput.log"));
Process p2 = pb2.start();
int exit2 = p2.waitFor();

it fails with the following error:

Error: Unable to initialize main class fr.inria.astor.core.validation.junit.JUnitNologExternalExecutor
Caused by: java.lang.NoClassDefFoundError: org/junit/runners/model/InitializationError

Despite the astor classes being in classpath.

In general, the Launcher looks a bit scary. Can you help me understand why there is magic with stdin and stdout instead of creating tempfiles? And why is the timezone fixed to LA?

====================================================

On the other side the -properties gzoltarpath:/apps/astor/lib/gzoltar does not work as described. Do I have to make entries in the ProjectProperties?

@martinezmatias
Copy link
Collaborator

Hi @lapplislazuli

why is the timezone fixed to LA?

I imagine that the timezone was set to LA because Defects4J required that.

the Launcher looks a bit scary. Can you help me understand why there is magic with stdin and stdout instead of creating tempfiles?

The idea -at that time- was that the JunitNoLogExecutor runs the test cases and returns the results (i.e. number of total executed test and number of failing/passing). The challenge was to communicate that class with the rest of Astor (i.e. the LaucherJUnitProcess). The simplest way I found and that time (but not sure if it was the best) it was that JunitNoLogExecutor prints the results on stdout and LaucherJUnitProcess parsers the output from JunitNoLogExecutor process to get the results (see here)
We could put the output on files, however I thought that writing and reading files each time we run test cases could harm the performance.

On the other side the -properties gzoltarpath:/apps/astor/lib/gzoltar does not work as described. Do I have to make entries in the ProjectProperties?

Sorry, the argument is -parameters, not -properties: https://github.com/SpoonLabs/astor/blob/master/docs/arguments.md#astor-parameters

Regards
Matias

@lapplislazuli
Copy link
Author

Good Morning @martinezmatias ,

I am gravely stuck by now 😅

Good things first: The -parameter worked properly.

In general I understand what's going on in the JunitExecutor, but it seems that the attributes it tries to put in do not align anymore. Usually some class is missing, that can either be a non-found test-class or a junit-dependency missing.
When I manually check the CP, it is usually fine, at least I do not see anything's missing.

So I fiddled with the JunitExecutor manually (try to run it with the command on console) and one thing that I noticed is the provided command tries to run single tests, as in test-methods not test-classes.
The other thing is, if I manually change it, it "works" but the output is rather cryptic such as

astoroutdel221astoroutdel1astoroutdelastoroutdel

I saw that astoroutdel is set as the delimiter, but what is 221 - 1 supposed to mean?

Can you have a look at the current state of the PR ?
I feel a little lost where to look for the bits and bolts missing, or what to redo/rewrite.

@martinezmatias
Copy link
Collaborator

martinezmatias commented May 7, 2021

Hi @martinezmatias

I saw that astoroutdel is set as the delimiter, but what is 221 - 1 supposed to mean?

221 is the number of test cases executed, `1' is the number of failing test cases.
That string is parsed here:

int nrtc = Integer.valueOf(resultPrinted[1]);

I noticed is the provided command tries to run single tests

Astor tries to run first the class(es) that include failing tests.
Then, if all of them passes, it executes all tests from the app.

Can you have a look at the current state of the PR ?

First of all, thanks for the many improvements introduced. The main problem is see in the PR is that it covers several issues: e.g., upgrate Gzoltar + dockerfile + fixes to be able to run on Docker.
I will propose to create one particular PR for each feature. We can start with the Gzoltar issue, and once we merge we continue with the other points.
WDYT?

Thanks!
Matias

@lapplislazuli
Copy link
Author

@martinezmatias thanks for the quick response.
I think it's actually all still connected to migrating above java 8 in #307 .
All of these changes are necessary to actually run it above java 8, they do not make a lot of sense on their own.
The only thing that's a bit standalone is the dockerfile, but that's nice to show the same issues for this PR across multiple machines.

@monperrus
Copy link
Contributor

FTR, Astor now supports GZoltar 1.7, hence the conflicts here, see master

@monperrus
Copy link
Contributor

closing stale pull requests, thanks again @lapplislazuli

@monperrus monperrus closed this Aug 7, 2024
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.

4 participants