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

Addition of 43 Closure faults #167

Merged
merged 70 commits into from Feb 27, 2019

Conversation

Projects
None yet
3 participants
@Greg4cr
Copy link
Collaborator

Greg4cr commented Jul 25, 2018

These 43 faults were omitted during the original creation of Defects4J as they were large patches. We have elected to include them now.

@Greg4cr Greg4cr requested a review from rjust Jul 25, 2018

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Aug 16, 2018

FYI - update from yesterday's conversation. TravisCI still failed due to the flaky Mockito relevant tests, but the Closure elements of this are passing. I think we could merge. Double check and let me know if it's fine.

@jose

This comment has been minimized.

Copy link
Collaborator

jose commented Aug 16, 2018

Hi @Greg4cr, thanks for the pull request. Were these new 43 faults reported after D4J v0.0.1? If not, any clue why weren't they include in v0.0.1?

And, btw, did you manually minimise the source patches, e.g., removed any cosmetic changes? At first glance they look minimal to me, but I would have to check in detail each one to be sure.

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Aug 16, 2018

@jose These 43 defects are part of an ongoing project with @rjust on patch minimization. They were rejected from D4J v0.0.1 for being likely impure patches containing both cosmetic and bug-fixing changes. We have decided to manually minimize them and include them now to add examples to D4J that were not largely-minimal in the first place. @rjust is also working on manually minimizing the initially-rejected faults for Chart, Lang, Math, and Time as part of the same project.

I have manually minimized these to remove cosmetic changes, and they should be in good shape. Of course, additional minimalization is welcome if you spot any obvious changes to make.

@jose

This comment has been minimized.

Copy link
Collaborator

jose commented Aug 16, 2018

cool, I wasn't aware of such project on patch minimization.

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Aug 16, 2018

@jose Not a problem!

@Greg4cr Greg4cr added the enhancement label Aug 27, 2018

@Greg4cr Greg4cr requested a review from jose Sep 19, 2018

@jose

This comment has been minimized.

Copy link
Collaborator

jose commented Sep 24, 2018

Hi @Greg4cr,

Quick question. Which java version did you use to generate the trigger_tests files? I'm currently using openjdk-1.7.0_80-b15 and I'm getting different stack traces...

--
Cheers,
Jose

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Sep 24, 2018

I probably used JDK8 to generate and then checked that the same tests failed in JDK7. Feel free to replace the stack traces if you want.

@jose

This comment has been minimized.

Copy link
Collaborator

jose commented Oct 1, 2018

Hi @Greg4cr,

I've just found an interesting case that I would like to discuss with you.

Although the test case com.google.javascript.jscomp.GraphColoringTest::testDeterministic (or com.google.javascript.jscomp.graph.GraphColoringTest::testDeterministic) is considered as a non-valid failing test case by D4J and therefore it is discarded for bugs 98, 99, 100, 101, 102, 103, 104, and 105; it is considered (according to your data) as a triggering test case for bugs 156, 157, 158, 160, 161, 162, 163, 164, 165, 166, 167, 168, and 169.

In order to check whether it is indeed a triggering test case I ran it on the fixed version of those bugs:

$ /tmp
$ pid=Closure
$ for bid in 156 157 158 160 161; do \
     rm -rf "$pid-${bid}f" && \
     $D4J_HOME/framework/bin/defects4j checkout -p "$pid" -v "${bid}f" -w "$pid-${bid}f" && \
     cd "$pid-${bid}f" && \
     $D4J_HOME/framework/bin/defects4j test -t com.google.javascript.jscomp.graph.GraphColoringTest::testDeterministic; \
     cd /tmp; done
$ for bid in 162 163 164 165 166 167 168 169; do \
     rm -rf "$pid-${bid}f" && \
     $D4J_HOME/framework/bin/defects4j checkout -p "$pid" -v "${bid}f" -w "$pid-${bid}f" && \
     cd "$pid-${bid}f" && \
     $D4J_HOME/framework/bin/defects4j test -t com.google.javascript.jscomp.GraphColoringTest::testDeterministic; \
     cd /tmp; done

com.google.javascript.jscomp.graph.GraphColoringTest::testDeterministic does fail when executed on the fixed version of bugs 156, 157, 158, 160, and 161; and com.google.javascript.jscomp.GraphColoringTest::testDeterministic fails when executed on the fixed version of bugs 162, 163, 164, 165, 166, 167, 168, and 169. Should not both test cases be considered as a non-valid failing test case and therefore be included in the defects4j/framework/projects/Closure/failing_tests/__revision_id__ file?

PS: Either com.google.javascript.jscomp.graph.GraphColoringTest::testDeterministic or com.google.javascript.jscomp.GraphColoringTest::testDeterministic might also fail when executed on the fixed version of other Closure bugs (i.e., 134-176), I haven't check that yet.

--
Best,
Jose

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Oct 1, 2018

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Oct 1, 2018

@jose

This comment has been minimized.

Copy link
Collaborator

jose commented Oct 1, 2018

Got it. Both test cases (com.google.javascript.jscomp.graph.GraphColoringTest::testDeterministic and com.google.javascript.jscomp.GraphColoringTest::testDeterministic) do not fail when I use Java-8, neither on the fixed or buggy version. Is there any chance you ran the bug-mining on Java-8 and then the analysis of the triggering test cases on Java-7? That could explain why they were considered triggering test cases and not failing test cases.

I'm going to automatically regenerate the failing_tests, trigger_tests, relevant_tests, and loaded_classes files using the original patch and Java-7, and I will then manually review whether the patches you minimised are indeed minimal. We should be good to merge this after that.

---8<----

I might have found another (potential) issue. :-/
The patch file of bugs 152 and 156 includes non java files. Due to that, the modified_classes/152.src and modified_classes/156.src include non java classes names.

152.src.patch

diff --git a/src/com/google/javascript/jscomp/parsing/ParserConfig.properties b/src/com/google/javascript/jscomp/parsing/ParserConfig.properties
index 9285339d..a7493162 100644
--- a/src/com/google/javascript/jscomp/parsing/ParserConfig.properties
+++ b/src/com/google/javascript/jscomp/parsing/ParserConfig.properties
@@ -26,10 +26,8 @@
 // The version of the compiler that we're currently building.
 // Should be formatted as:
 // Version# (Revision XXX)
-compiler.version = HEAD
 
 // The datestamp of the compiler that we're currently building.
-compiler.date = NOW
 
 // A comma-delimited list.
 jsdoc.annotations =\

This change does not seem related to the bug, i.e., the list of triggering test cases is the same either with or without this change.

156.src.patch

diff --git a/src/com/google/javascript/jscomp/js/runtime_type_check.js b/src/com/google/javascript/jscomp/js/runtime_type_check.js
index 87c94834..fdb7c09d 100644
--- a/src/com/google/javascript/jscomp/js/runtime_type_check.js
+++ b/src/com/google/javascript/jscomp/js/runtime_type_check.js
@@ -97,13 +97,13 @@ jscomp.typecheck.prettify_ = function(expr) {
  * @private
  */
 jscomp.typecheck.getClassName_ = function(expr) {
-  var className = void 0;
+  var className = undefined;
   if (typeof expr == 'object' && expr && expr.constructor) {
     className = expr.constructor.name;
     if (!className) {
       var funNameRe = /function (.{1,})\(/;
       var m = (funNameRe).exec(expr.constructor.toString());
-      className = m && m.length > 1 ? m[1] : void 0;
+      className = m && m.length > 1 ? m[1] : undefined;
     }
   }
   return className;

This change does seem related to the bug, i.e., if I remove it from the patch I get a different number of triggering test cases.

@rjust, is this something we would consider as correct? Or, does this reveal an issue in the bug-mining process?

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Oct 1, 2018

@jose

jose approved these changes Feb 19, 2019

Copy link
Collaborator

jose left a comment

This pull request has passed all my sanity checks.

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Feb 19, 2019

@rjust @jose Shall we merge?

@rjust

This comment has been minimized.

Copy link
Owner

rjust commented Feb 22, 2019

How are the new Closure bugs ordered in the commit-db?

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Feb 22, 2019

@rjust - They are 144 - 176.

@rjust

This comment has been minimized.

Copy link
Owner

rjust commented Feb 22, 2019

Right, but it seems that the commit-db for these bugs is not ordered in any particular way. Shouldn't the new bugs be ordered chronologically (old to new)?

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Feb 22, 2019

That was what I would have expected.

The order is based on the commit-db you provided originally. I attached it here. Was there an early point in the project where batches of bugs were added non-chronologically? Maybe from different regex or something?

commit-db.txt

@rjust

This comment has been minimized.

Copy link
Owner

rjust commented Feb 22, 2019

It's possible that the list of candidates was in no particular order (or in a weird one), but the promote script should take care of this while re-enumerating the bug IDs. Is this updated commit-db generated by the promote script? If not, should we run it through this script?

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Feb 22, 2019

That would be the issue. I did all of this work before the existence of the current promote script.

@rjust

This comment has been minimized.

Copy link
Owner

rjust commented Feb 22, 2019

That makes sense. Let's put these in the right order, and then we are good to go.

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Feb 22, 2019

Bugs are reordered.

@jose

This comment has been minimized.

Copy link
Collaborator

jose commented Feb 25, 2019

How are the new Closure bugs ordered in the commit-db?

The new mined bugs are ordered as they appear (as fixed) in the commit history, from the oldest to the newest.

@jose
Copy link
Collaborator

jose left a comment

Hi @Greg4cr,

Quick question, why have you modified the metadata files of the new bugs?

--
Best,
Jose

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Feb 25, 2019

The new mined bugs are ordered as they appear (as fixed) in the commit history, from the oldest to the newest.

@jose - This is not actually true for these bugs. The reason for this is that the bug mining framework was NOT used to create the commit-db for the new Closure bugs. Rather, an old commit-db was used, from when Defects4J was first created. This commit-db was not sorted in the same way that newly mined projects would be.

As a result, the bugs were not actually ordered. Rather, there were two broad groupings:

  • Bugs 134 - 169 represent bugs from roughly 2009-2011, and are ordered from newest to oldest.
  • Bugs 170 - 176 represent a new group from 2013. These are mostly ordered from newest to oldest, except that 170 and 171 were in the wrong order.

My latest commit reorders all of these, sorted chronologically from oldest to newest. This is why the metadata has changed. The individual bugs have the same metadata they used to, the order has just shifted.

@jose

This comment has been minimized.

Copy link
Collaborator

jose commented Feb 25, 2019

@jose - This is not actually true for these bugs. The reason for this is that the bug mining framework was NOT used to create the commit-db for the new Closure bugs. Rather, an old commit-db was used, from when Defects4J was first created. This commit-db was not sorted in the same way that newly mined projects would be.

But I believe I rebuilt the commit-db using the latest version of the bug mining framework...

@jose

This comment has been minimized.

Copy link
Collaborator

jose commented Feb 25, 2019

hmm, something is not right in the bug-mining framework. according to the git log, the issue 74 was fixed in 2010 and issue 791 was fixed in 2012... :-/
let me try to re-generate the commit_db.

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Feb 25, 2019

But I believe I rebuilt the commit-db using the latest version of the bug mining framework...

You may have, but it did not change the order in the commit-db. I just compared the commit-db to the old one I used initially, and the ordering is the same. If you run git log on the fix commits, you would see that the ordering is not chronological.

Fix Commit Dates (from before my last commit):
Date: Fri Oct 19 16:41:16 2012 +0000
Date: Fri Aug 10 19:49:41 2012 +0000
Date: Thu Aug 2 22:05:42 2012 +0000
Date: Wed Aug 1 23:56:31 2012 +0000
Date: Mon May 21 15:58:30 2012 +0000
Date: Tue Feb 7 18:37:27 2012 +0000
Date: Tue Dec 6 19:03:54 2011 +0000
Date: Tue Sep 6 23:59:07 2011 +0000
Date: Tue Aug 2 19:34:59 2011 +0000
Date: Fri May 27 17:34:04 2011 +0000
Date: Thu Apr 14 17:48:58 2011 +0000
Date: Wed Apr 6 18:16:30 2011 +0000
Date: Mon Apr 4 21:26:33 2011 +0000
Date: Tue Mar 29 23:46:31 2011 +0000
Date: Sat Mar 26 00:16:46 2011 +0000
Date: Mon Feb 28 16:08:39 2011 +0000
Date: Thu Dec 9 20:58:52 2010 +0000
Date: Thu Oct 28 02:03:31 2010 +0000
Date: Fri Sep 17 19:56:20 2010 +0000
Date: Tue Sep 14 00:41:11 2010 +0000
Date: Fri Aug 6 02:38:20 2010 +0000
Date: Thu Jul 15 21:01:20 2010 +0000
Date: Fri Jul 9 23:02:36 2010 +0000
Date: Wed Jul 7 22:14:18 2010 +0000
Date: Fri Jun 25 23:56:52 2010 +0000
Date: Wed Jun 23 00:19:37 2010 +0000
Date: Wed Apr 7 18:34:02 2010 +0000
Date: Tue Apr 6 06:58:16 2010 +0000
Date: Thu Mar 25 15:58:20 2010 +0000
Date: Tue Mar 23 05:51:47 2010 +0000
Date: Mon Mar 22 22:55:34 2010 +0000
Date: Thu Mar 18 23:07:23 2010 +0000
Date: Thu Mar 18 22:51:03 2010 +0000
Date: Tue Mar 16 22:53:04 2010 +0000
Date: Fri Feb 5 18:31:28 2010 +0000
Date: Fri Jan 8 21:41:36 2010 +0000
Date: Sun Nov 3 15:55:39 2013 -0500
Date: Mon Nov 4 10:58:43 2013 -0800
Date: Sat Sep 28 15:18:56 2013 -0400
Date: Tue Aug 20 11:15:56 2013 -0700
Date: Mon Jul 15 16:05:31 2013 -0700
Date: Tue Jun 18 02:52:07 2013 -0700
Date: Wed Apr 17 13:47:23 2013 -0700

Fix Commit Dates (after changes):
Date: Fri Jan 8 21:41:36 2010 +0000
Date: Fri Feb 5 18:31:28 2010 +0000
Date: Tue Mar 16 22:53:04 2010 +0000
Date: Thu Mar 18 22:51:03 2010 +0000
Date: Thu Mar 18 23:07:23 2010 +0000
Date: Mon Mar 22 22:55:34 2010 +0000
Date: Tue Mar 23 05:51:47 2010 +0000
Date: Thu Mar 25 15:58:20 2010 +0000
Date: Tue Apr 6 06:58:16 2010 +0000
Date: Wed Apr 7 18:34:02 2010 +0000
Date: Wed Jun 23 00:19:37 2010 +0000
Date: Fri Jun 25 23:56:52 2010 +0000
Date: Wed Jul 7 22:14:18 2010 +0000
Date: Fri Jul 9 23:02:36 2010 +0000
Date: Thu Jul 15 21:01:20 2010 +0000
Date: Fri Aug 6 02:38:20 2010 +0000
Date: Tue Sep 14 00:41:11 2010 +0000
Date: Fri Sep 17 19:56:20 2010 +0000
Date: Thu Oct 28 02:03:31 2010 +0000
Date: Thu Dec 9 20:58:52 2010 +0000
Date: Mon Feb 28 16:08:39 2011 +0000
Date: Sat Mar 26 00:16:46 2011 +0000
Date: Tue Mar 29 23:46:31 2011 +0000
Date: Mon Apr 4 21:26:33 2011 +0000
Date: Wed Apr 6 18:16:30 2011 +0000
Date: Thu Apr 14 17:48:58 2011 +0000
Date: Fri May 27 17:34:04 2011 +0000
Date: Tue Aug 2 19:34:59 2011 +0000
Date: Tue Sep 6 23:59:07 2011 +0000
Date: Tue Dec 6 19:03:54 2011 +0000
Date: Tue Feb 7 18:37:27 2012 +0000
Date: Mon May 21 15:58:30 2012 +0000
Date: Wed Aug 1 23:56:31 2012 +0000
Date: Thu Aug 2 22:05:42 2012 +0000
Date: Fri Aug 10 19:49:41 2012 +0000
Date: Fri Oct 19 16:41:16 2012 +0000
Date: Wed Apr 17 13:47:23 2013 -0700
Date: Tue Jun 18 02:52:07 2013 -0700
Date: Mon Jul 15 16:05:31 2013 -0700
Date: Tue Aug 20 11:15:56 2013 -0700
Date: Sat Sep 28 15:18:56 2013 -0400
Date: Sun Nov 3 15:55:39 2013 -0500
Date: Mon Nov 4 10:58:43 2013 -0800

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Feb 25, 2019

Interestingly, the old Closure bugs are not entirely chronological either, if you go by the git log.

@jose

This comment has been minimized.

Copy link
Collaborator

jose commented Feb 25, 2019

I've just re-executed the bug-mining framework and it does build a similar commit_db file. The only difference is:

Current commit_db file

...
175,d6eebf3357f6f06b44cefbe55aabeab5bf25216e,aeed47f424d93d9ff82e0782fca53259829362b1,1056,https://storage.googleapis.com/google-code-archive/v2/code.google.com/closure-compiler/issues/issue-1056.json
176,94623ace4074dea70ffdde117a2d5e11c76de5fa,038da2119223818ee1c56eaf600a583f755f9b30,1101,https://storage.googleapis.com/google-code-archive/v2/code.google.com/closure-compiler/issues/issue-1101.json

vs. commit_db file generated by the bug-mining framework

...
175,94623ace4074dea70ffdde117a2d5e11c76de5fa,038da2119223818ee1c56eaf600a583f755f9b30,1101,https://storage.googleapis.com/google-code-archive/v2/code.google.com/closure-compiler/issues/issue-1101.json
176,d6eebf3357f6f06b44cefbe55aabeab5bf25216e,aeed47f424d93d9ff82e0782fca53259829362b1,1056,https://storage.googleapis.com/google-code-archive/v2/code.google.com/closure-compiler/issues/issue-1056.json

Issue 1056 was fixed on Nov 3 15:55:39 2013 (aeed47f424d93d9ff82e0782fca53259829362b1), and issue 1101 was fixed on Nov 4 10:58:43 2013 (038da2119223818ee1c56eaf600a583f755f9b30). (Please note that there are two other commits from Oct 1 2013 that also mention issue 1101). Although issue 1056 was fixed a day before issue 1101, its fix message appears after in the repository history, only on Nov 18 2013.

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Feb 25, 2019

Interesting. I'm willing to go with your version of the 175/176 ordering then.

@jose

This comment has been minimized.

Copy link
Collaborator

jose commented Feb 25, 2019

@Greg4cr could you please make that change?

@Greg4cr

This comment has been minimized.

Copy link
Collaborator Author

Greg4cr commented Feb 25, 2019

@jose - done.

@jose

This comment has been minimized.

Copy link
Collaborator

jose commented Feb 25, 2019

Thanks @Greg4cr. @rjust, any other question/comment?

@rjust

rjust approved these changes Feb 27, 2019

@Greg4cr Greg4cr merged commit 1b56b59 into rjust:master Feb 27, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.