Merge pull request #74 from adamtornhill/57-specify-pre-condition-for-messages-analysis

Enforce the pre-conditions on the messages analysis

Author: adamtornhill

src/code_maat/analysis/commit_messages.clj

@@ -32,11 +32,6 @@
      (IllegalArgumentException.
       "Commit messages: you need to provide an expression to match against."))))
 
-(defn- columns-of-interest
-  "Filter away all the data we won't need (less GC overhead)."
-  [ds]
-  (dataset/-select-by [:entity :message] ds))
-
 (defn- commit-matches
   "Performs a match for the expression provided by the
    user against a single commit message."
@@ -55,6 +50,22 @@
    frequencies
    (into [])))
 
+(defn- commit-log-without-messages?
+  [ds]
+  (let [ncommits-with-message (->> ds (dataset/-where {:message  {:$fn (fn [m] (not= m "-"))}}) dataset/-nrows)
+        nrows (dataset/-nrows ds)]
+    (and (pos? nrows)
+         (= 0 ncommits-with-message))))
+
+(defn- ensure-supported-vcs
+  "The git2 format doesn't support this analysis since the commit messages aren't included. Here we make a more
+   general check."
+  [ds]
+  (if (commit-log-without-messages? ds)
+    (throw (IllegalArgumentException. (str "Wrong version-control format. Cannot do a messages analysis without commit messages. "
+                                           "Look at the difference between the git and git2 formats in the docs.")))
+    ds))
+
 (defn by-word-frequency
   "Returns the frequencies of the given word matches
    across all entities.
@@ -65,7 +76,9 @@
    this function counts the occourences."
   [ds options]
   (->>
-   (rows-matching-given-expr (match-expr-from options) ds)
-   as-matching-entity-freqs
-   (incanter/dataset [:entity :matches])
-   (dataset/-order-by [:matches :entity] :desc)))
+    ds
+    ensure-supported-vcs
+    (rows-matching-given-expr (match-expr-from options))
+    as-matching-entity-freqs
+    (incanter/dataset [:entity :matches])
+    (dataset/-order-by [:matches :entity] :desc)))

src/code_maat/app/app.clj

@@ -210,6 +210,8 @@
     (output-fn! (analysis-fn changes))
     (catch AssertionError e ; typically a pre- or post-condition
       (throw-internal-error e))
+    (catch IllegalArgumentException e
+      (throw e))
     (catch Exception e
       (throw-internal-error e))))
 

test/code_maat/analysis/commit_messages_test.clj

@@ -6,7 +6,8 @@
 (ns code-maat.analysis.commit-messages-test
   (:require [code-maat.analysis.commit-messages :as c]
             [code-maat.analysis.test-data :as td]
-             [code-maat.dataset.dataset :as ds])
+            [code-maat.dataset.dataset :as ds]
+            [incanter.core :as incanter])
   (:use clojure.test))
 
 (defn- as-option
@@ -23,4 +24,26 @@
   (is (= (c/by-word-frequency td/vcsd (as-option "no match for this"))
          (ds/-dataset [:entity :matches]
                       []))))
+
+(defn- run-messages-analysis
+  [commits]
+  (-> commits
+      incanter/to-dataset
+      (c/by-word-frequency (as-option "change"))))
+
+(deftest detects-absent-message-fields ; not all supported version-control formats include a commit message
+  (testing "No commit messages triggers exception"
+    (is (thrown? IllegalArgumentException
+                 (run-messages-analysis [{:author "apt" :entity "A" :rev 1 :message "-"}
+                                         {:author "apt" :entity "B" :rev 2 :message "-"}]))))
+  (testing "Empy dataset is valid"
+    (is (= (run-messages-analysis [])
+           (ds/-dataset [:entity :matches]
+                        []))))
+  (testing "Valid, if any commit has a message"
+    (is (= (run-messages-analysis [{:author "apt" :entity "A" :rev 1 :message "-"}
+                                   {:author "apt" :entity "B" :rev 2 :message "some change message"}])
+           (ds/-dataset [:entity :matches]
+                        [["B" 1]])))))
+
 

test/code_maat/end_to_end/scenario_tests.clj

@@ -106,6 +106,15 @@
   (is (= (run-with-str-output log-file options)
          "author,peer,shared,average,strength\nXYZ,APT,1,2,50\nAPT,XYZ,1,2,50\n")))
 
+(defn- with-message-to-match
+  [options msg]
+  (assoc options :expression-to-match  msg))
+
+(deftest counts-commit-message-patterns
+  (is (= (run-with-str-output git-log-file
+                              (with-message-to-match (git-options "messages")"stat"))
+         "entity,matches\n/Infrastrucure/Network/Connection.cs,1\n")))
+
 ;;; The identity analysis is intended as a debug aid or to
 ;;; generate parsed VCS data as input to other tools.
 ;;; The idea with identity is to dump the parse result to
@@ -144,7 +153,7 @@
 
 (deftest reports-invalid-arguments
   (testing "Non-existent input file"
-    (is (thrown? IllegalArgumentException (app/run "I/do/not/exist"))))
+    (is (thrown? IllegalArgumentException (app/run "I/do/not/exist" {}))))
   (testing "Missing mandatory options (normally added by the front)"
     (is (thrown? IllegalArgumentException (app/run svn-log-file {:analysis "coupling"})))))