initial commit for iFixFlakies4iFixPlus #24

ChopinLi-cp · 2022-02-21T17:38:18Z

No description provided.

august782 · 2022-03-09T18:57:48Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+import org.xmlunit.diff.Difference;
+import org.xmlunit.diff.ElementSelectors;
+
+public class iFixPlusPlugin extends TestPlugin {


Maybe we should change this to something like ODRepairPlugin, or ODRepairDebugPlugin, as to better match the paper.

august782 · 2022-03-09T18:58:23Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+
+public class iFixPlusPlugin extends TestPlugin {
+    private Path replayPath;
+    private Path replayPath2;


Can we use a better name than replayPath2 as to distinguish what it is actually supposed to represent?

august782 · 2022-03-09T18:59:58Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+        final Runner runner = InstrumentingSmartRunner.fromRunner(runners.get(0));
+
+        replayPath = Paths.get(Configuration.config().getProperty("replay.path"));
+        replayPath2 = Paths.get(Configuration.config().getProperty("replay.path2"));


Similar thing about the property name itself. In fact, should they all have the prefix "replay"? If they are more relevant for controlling testrunner's statecapture logic, maybe they should instead be prefix "statecapture"?

august782 · 2022-03-09T19:01:19Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+                // phase 0 check json file
+                System.out.println("~~~~~~ Begin to check if this test is a true order-dependent test.");
+                Configuration.config().properties().setProperty("statecapture.phase", "check");
+                if (testFailOrder()==null) {


Spaces between operators, i.e., testFailOrder() == null.

august782 · 2022-03-09T19:02:42Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+                        StandardOpenOption.APPEND);
+                // phase 0 check json file
+                System.out.println("~~~~~~ Begin to check if this test is a true order-dependent test.");
+                Configuration.config().properties().setProperty("statecapture.phase", "check");


Does configuring the phase to be "check" affect anything for the testrunner run itself?

august782 · 2022-03-12T05:24:14Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+        System.out.println("xmlFileName: " + xmlFileNum);
+
+        if (runner != null && module.equals(PathManager.modulePath().toString())) {
+            System.out.println("replyPath: " + replayPath);


Can remove such print statements; also, replyPath -> replayPath.

august782 · 2022-03-12T05:24:44Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+
+    @Override
+    public void execute(final MavenProject mavenProject) {
+        Configuration.config().properties().setProperty("statecapture.phase", "initial");


Does setting the phase to be "initial" affect anything?

august782 · 2022-03-12T05:25:27Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+                }
+
+                for(int i=0; i<10; i++) {
+                    Try<TestRunResult> phase0ResultFail = null;


Let's also change the variable names to not use numbered phases, to match exactly what phase we care about.

august782 · 2022-03-12T05:26:05Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+                        phase0ResultFail = runner.runList(testFailOrder());
+                    }
+                    catch (Exception ex) {
+                        System.out.println("## Encountering error when checking the failing order: " + ex);


What kind of exceptions can we get there?

august782 · 2022-03-12T05:27:28Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+                    System.out.println("Failing order results: " +
+                            phase0ResultFail.get().results().get(dtname).result().toString());
+
+                    if (phase0ResultFail.get().results().get(dtname).result().toString().equals("PASS")) {


Since the variable gets initalized to null and can potentially not be set properly in the try block, this could actually lead to a NullPointerException. That kind of defeats the purpose of using the Try construct, since the point of that is to not have to check for null (you would first check whether it has a value).

august782 · 2022-03-12T05:28:22Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+                    System.out.println("## Running double victim order results: " + phase1Result.get().results());
+                }
+                catch (Exception e) {
+                    System.out.println("## Encountering error when running in double victim order: " + e);


Please check what kind of exceptions we can get in these places.

august782 · 2022-03-12T05:29:12Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+
+        replayPath = Paths.get(Configuration.config().getProperty("replay.path"));
+        replayPath2 = Paths.get(Configuration.config().getProperty("replay.path2"));
+        dtname= Configuration.config().getProperty("replay.dtname");


Spaces after the variable name.

august782 · 2022-03-12T05:30:01Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+                    System.out.println("~~~~~~ Finish phase capturing the state in passing order(double victim)!!");
+                }
+
+                xmlFileNum = countDirNums(subxmlFold);


Do we still need to do these counting?

august782 · 2022-03-12T05:30:39Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+                    Configuration.config().properties().
+                            setProperty("statecapture.phase", "capture_before");
+                    Configuration.config().properties().
+                            setProperty("statecapture.state", "passing_order");


Is this "statecapture.state" property actually useful?

august782 · 2022-03-12T05:31:31Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+                    System.out.println("~~~~~~ Begin capturing the state in passing order!");
+                    // phase 2: run doublevictim order state capture
+                    Configuration.config().properties().
+                            setProperty("statecapture.phase", "capture_after");


I wonder instead of "phase" the property should actually reflect that it means the relative position for capture, maybe something like "statecapture.position"?

august782 · 2022-03-12T05:32:30Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+                xmlFileNum = countDirNums(subxmlFold);
+                System.out.println("xmlFileNum: " + xmlFileNum);
+
+                File passingSubXmlFolder = new File(subxmlFold + "/failing_order_xml");


Why is passingSubXmlFolder referencing the name "failing_order_xml"?

august782 · 2022-03-12T05:33:36Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+                        String line;
+                        while ((line = br.readLine()) != null) {
+                            if (line.contains(" made test success#######")) {
+                                successfulField = line.substring(8, line.lastIndexOf(" made test success#######"));


This feels so complicated to parse some English, can we get it to simply write out the valid polluted field?

august782 · 2022-03-12T05:35:54Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+        return num;
+    }
+
+    public void diffing() {


If all diffing does is call diffSub, is there any need for this method?

august782 · 2022-03-12T05:37:31Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+        return FileUtils.readFileToString(file, "UTF-8");
+    }
+
+    Set<String> File2SetString(String path) {


Method should not start with capitalized letter, should have access modifier like private, maybe change to something like readFileContentsAsSet.

august782 · 2022-03-12T05:41:09Z

src/main/java/edu/illinois/cs/dt/tools/utility/iFixPlusPlugin.java

+                }
+
+                if (!state0.equals(state1)) {
+                    diffFields_filtered.add(s);


Does this just compare whether the two strings read from the files are exactly the same? Shouldn't there be a more in-depth comparison to determine whether they are truly different?

initial commit for iFixFlakies4iFixPlus

93a152d

ChopinLi-cp force-pushed the iFixFlakies4iFixPlus branch from 90b4245 to a243828 Compare February 24, 2022 22:55

move the diff part to iFixPlusPlugin and refactor the code

bcb03f8

ChopinLi-cp force-pushed the iFixFlakies4iFixPlus branch from a243828 to bcb03f8 Compare February 24, 2022 22:58

ChopinLi-cp added 2 commits March 5, 2022 22:18

change the phase

9b33e52

remove the polluter and unuseful code

728f744

august782 reviewed Mar 9, 2022

View reviewed changes

august782 reviewed Mar 12, 2022

View reviewed changes

ChopinLi-cp added 3 commits March 16, 2022 16:09

refactoring the code to only refer the file through configuration

d0ebf43

commit the iFixPlusPlugin file

33e7c0c

change the iFixPlusPlugin accordingly

4caef13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

initial commit for iFixFlakies4iFixPlus #24

initial commit for iFixFlakies4iFixPlus #24

ChopinLi-cp commented Feb 21, 2022

august782 Mar 9, 2022

august782 Mar 9, 2022

august782 Mar 9, 2022

august782 Mar 9, 2022

august782 Mar 9, 2022

august782 Mar 12, 2022

august782 Mar 12, 2022

august782 Mar 12, 2022

august782 Mar 12, 2022

august782 Mar 12, 2022

august782 Mar 12, 2022

august782 Mar 12, 2022

august782 Mar 12, 2022

august782 Mar 12, 2022

august782 Mar 12, 2022

august782 Mar 12, 2022

august782 Mar 12, 2022

august782 Mar 12, 2022

august782 Mar 12, 2022

august782 Mar 12, 2022

initial commit for iFixFlakies4iFixPlus #24

Are you sure you want to change the base?

initial commit for iFixFlakies4iFixPlus #24

Conversation

ChopinLi-cp commented Feb 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment