initial commit for iFixFlakies4iFixPlus by ChopinLi-cp · Pull Request #24 · TestingResearchIllinois/iFixFlakies

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

No description provided.

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

+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

+
+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

+        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

+                // 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

+                        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

+        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

+
+    @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

+                }
+
+                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

+                        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

+                    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

+                    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

+
+        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

+                    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

+                    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

+                    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

+                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

+                        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

+        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

+        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

+                }
+
+                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

Conversation

ChopinLi-cp commented Feb 21, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants