A small refactoring with a few unit tests

andygrunwald · andygrunwald · commit 18203c102f9c · 2017-11-28T16:09:29.000+01:00
diff --git a/README.md b/README.md
@@ -18,19 +18,19 @@
 ```
 Usage of ./jitic:
   -issues string
-    	Message to retrieve the issues from.
+        Message to retrieve the issues from.
   -pass string
-    	JIRA Password.
+        JIRA Password.
   -stdin
-    	If set to true you can stream "-issues" to stdin instead of an argument. If set "-issues" will be ignored.
+        If set to true you can stream "-issues" to stdin instead of an argument. If set "-issues" will be ignored.
   -url string
-    	JIRA instance URL (format: scheme://[username[:password]@]host[:port]/).
+        JIRA instance URL (format: scheme://[username[:password]@]host[:port]/).
   -user string
-    	JIRA Username.
+        JIRA Username.
   -verbose
-    	If activated more information will be written to stdout .
+        If activated more information will be written to stdout .
   -version
-    	Outputs the version number and exits.
+        Outputs the version number and exits.
 ```
 
 ### Examples
@@ -52,8 +52,8 @@ Exit code: 0
 Check if a fake issue MESOS-123456 exists in [https://issues.apache.org/jira/](https://issues.apache.org/jira/) from parameter:
 
 ```bash
-$ ./jitic -url="https://issues.apache.org/jira/" -issues="MESOS-123456 - Not existing issue" -verbose
-2015/09/03 16:25:33 Issue MESOS-123456: 404 Not Found
+$ ./jitic -url="https://issues.apache.org/jira/" -issues="MESOS-123456 - Not existing issue" -verbose; echo "Exit code: $?"
+2017/11/28 15:41:26 JIRA Request for issue MESOS-123456 returned 404 Not Found (404)
 ```
 
 ## Use cases
diff --git a/main.go b/main.go
@@ -8,17 +8,14 @@ import (
 	"log"
 	"os"
 	"regexp"
+	"strings"
 
 	"gopkg.in/andygrunwald/go-jira.v1"
 )
 
 const (
-	// Version of jitic
-	Version = "0.2.2"
-)
-
-var (
-	logger *log.Logger
+	// Version reflects the version of jitic
+	Version = "1.0.0"
 )
 
 func main() {
@@ -33,90 +30,51 @@ func main() {
 	)
 	flag.Parse()
 
-	// Set logger (throw messages away)
-	logger = log.New(ioutil.Discard, "", log.LstdFlags)
+	// Set logger (throw messages away).
+	// In a verbose mode we output the messages to stdout
+	logger := log.New(ioutil.Discard, "", log.LstdFlags)
 	if *flagVerbose {
 		logger = log.New(os.Stdout, "", log.LstdFlags)
 	}
 
-	// Output the version and exit
+	// Output the version, exit
 	if *flagVersion {
 		fmt.Printf("jitic v%s\n", Version)
 		os.Exit(0)
 	}
 
-	// If we don`t get a JIRA instance, exit
+	// If we don`t have a JIRA instance URL, exit
 	if len(*jiraURL) <= 0 {
 		logger.Fatal("No JIRA Instance provided. Please set the URL of the JIRA instance by -url parameter.")
 	}
 
-	// Collect all issue keys
-	var issues []string
-	if len(*issueMessage) > 0 {
-		issues = GetIssuesOutOfMessage(*issueMessage)
-	}
+	issueText := getTextToAnalyze(*issueMessage, *inputStdin)
+	issues := getIssuesOutOfMessage(issueText)
 
-	// If we don`t get any issue, we will just exit here.
-	if *inputStdin == false && len(issues) == 0 {
-		logger.Fatal("No JIRA-Issue(s) found.")
+	// If we don`t get any issues, exit
+	if len(issues) == 0 {
+		logger.Fatalf("No JIRA-Issue(s) found in text '%s'.", issueText)
 	}
 
-	// Get the JIRA client
-	jiraInstance, err := jira.NewClient(nil, *jiraURL)
+	jiraClient, err := getJIRAClient(*jiraURL, *jiraUsername, *jiraPassword)
 	if err != nil {
-		logger.Fatalf("JIRA client can`t be initialized: %s", err)
+		logger.Fatal(err)
 	}
 
-	// Only provide authentification if a username and password was applied
-	if len(*jiraUsername) > 0 && len(*jiraPassword) > 0 {
-		ok, err := jiraInstance.Authentication.AcquireSessionCookie(*jiraUsername, *jiraPassword)
-		if ok == false || err != nil {
-			logger.Fatalf("jitic can`t authentificate user %s against the JIRA instance %s: %s", *jiraUsername, *jiraURL, err)
+	// Loop over all issues and check if they are correct / valid
+	for _, issueFromUser := range issues {
+		err := checkIfIssue(issueFromUser, jiraClient)
+		if err != nil {
+			logger.Fatal(err)
 		}
 	}
 
-	// If the issues will be applied by argument
-	if *inputStdin == false {
-		IssueLoop(issues, jiraInstance)
-	}
-
-	// If the issues will be applied by stdin
-	if *inputStdin {
-		ReadIssuesFromStdin(jiraInstance)
-	}
-
 	os.Exit(0)
 }
 
-// ReadIssuesFromStdin will read content vom standard input and search for JIRA issue keys
-// If an issue key was found a check with the incoming jiraInstance will be done.
-func ReadIssuesFromStdin(jiraInstance *jira.Client) {
-	scanner := bufio.NewScanner(os.Stdin)
-	for scanner.Scan() {
-		issues := GetIssuesOutOfMessage(scanner.Text())
-		// If no issue can be found
-		if len(issues) == 0 {
-			logger.Fatal("No JIRA-Issue(s) found.")
-		}
-		IssueLoop(issues, jiraInstance)
-	}
-}
-
-// IssueLoop will loop over issues and request jiraInstance to check if the issue exists.
-func IssueLoop(issues []string, jiraInstance *jira.Client) {
-	for _, incomingIssue := range issues {
-		issue, response, err := jiraInstance.Issue.Get(incomingIssue, nil)
-		if c := response.StatusCode; err != nil || (c < 200 && c > 299) {
-			logger.Fatalf("Issue %s: %s", incomingIssue, response.Status)
-		}
-		if incomingIssue != issue.Key {
-			logger.Fatalf("Issue %s is not the same as %s (provided by JIRA)", incomingIssue, issue.Key)
-		}
-	}
-}
-
-// GetIssuesOutOfMessage will retrieve all JIRA issue keys out of a text.
+// getIssuesOutOfMessage will retrieve all JIRA issue keys out of a text.
 // A text can be everything, but a use case is e.g. a commit message.
+//
 // Example:
 //		Text: WEB-22861 remove authentication prod build for now
 //		Result: WEB-22861
@@ -126,14 +84,11 @@ func IssueLoop(issues []string, jiraInstance *jira.Client) {
 //
 // @link https://confluence.atlassian.com/display/STASHKB/Integrating+with+custom+JIRA+issue+key
 // @link https://answers.atlassian.com/questions/325865/regex-pattern-to-match-jira-issue-key
-func GetIssuesOutOfMessage(issueMessage string) []string {
-	// Normally i would use
-	//		((?<!([A-Z]{1,10})-?)[A-Z]+-\d+)
-	// See http://stackoverflow.com/questions/26771592/negative-look-ahead-go-regular-expressions
+func getIssuesOutOfMessage(message string) []string {
 	var issues []string
 	re := regexp.MustCompile("(?i)([A-Z]+)-(\\d+)")
 
-	parts := re.FindAllStringSubmatch(issueMessage, -1)
+	parts := re.FindAllStringSubmatch(message, -1)
 	for _, v := range parts {
 		// If the issue number > 0 (to avoid matches for PSR-0)
 		if v[2] > "0" {
@@ -143,3 +98,68 @@ func GetIssuesOutOfMessage(issueMessage string) []string {
 
 	return issues
 }
+
+// checkIfIssue checks if issue exists in the JIRA instance.
+// If not an error will be returned.
+func checkIfIssue(issue string, jiraClient *jira.Client) error {
+	JIRAIssue, resp, err := jiraClient.Issue.Get(issue, nil)
+	if c := resp.StatusCode; err != nil || (c < 200 || c > 299) {
+		return fmt.Errorf("JIRA Request for issue %s returned %s (%d)", issue, resp.Status, resp.StatusCode)
+	}
+
+	// Make issues uppercase to be able to compare Web-1234 with WEB-1234
+	upperIssue := strings.ToUpper(issue)
+	upperJIRAIssue := strings.ToUpper(JIRAIssue.Key)
+	if upperIssue != upperJIRAIssue {
+		return fmt.Errorf("Issue %s is not the same as %s (provided by JIRA)", upperIssue, upperJIRAIssue)
+	}
+
+	return nil
+}
+
+// getJIRAClient will return a valid JIRA api client.
+func getJIRAClient(instanceURL, username, password string) (*jira.Client, error) {
+	client, err := jira.NewClient(nil, instanceURL)
+	if err != nil {
+		return nil, fmt.Errorf("JIRA client can`t be initialized: %s", err)
+	}
+
+	// Only provide authentification if a username and password was applied
+	if len(username) > 0 && len(password) > 0 {
+		ok, err := client.Authentication.AcquireSessionCookie(username, password)
+		if ok == false || err != nil {
+			return nil, fmt.Errorf("jitic can`t authentificate user %s against the JIRA instance %s: %s", username, instanceURL, err)
+		}
+	}
+
+	return client, nil
+}
+
+// readStdin will read content from stdin and return the content.
+func readStdin() string {
+	var text string
+	scanner := bufio.NewScanner(os.Stdin)
+	for scanner.Scan() {
+		text += scanner.Text()
+	}
+
+	return text
+}
+
+// getTextToAnalyze will return the text jitic should analyze.
+// From command line argument or/and stdin
+func getTextToAnalyze(argText string, inputStdin bool) string {
+	var text string
+
+	// We get a message via cmd argument
+	if len(argText) > 0 {
+		text = argText
+	}
+
+	// If stdin is activated
+	if inputStdin {
+		text += readStdin()
+	}
+
+	return text
+}
diff --git a/main_test.go b/main_test.go
@@ -1,10 +1,59 @@
 package main
 
 import (
+	"net/http"
+	"net/http/httptest"
 	"reflect"
 	"testing"
 )
 
+var (
+	// mux is the HTTP request multiplexer used with the test server.
+	mux *http.ServeMux
+
+	// server is a test HTTP server used to provide mock API responses.
+	server *httptest.Server
+)
+
+// setup sets up a test HTTP server along with a github.Client that is
+// configured to talk to that test server. Tests should register handlers on
+// mux which provide mock responses for the API method being tested.
+func setup() {
+	// test server
+	mux = http.NewServeMux()
+
+	// We want to ensure that tests catch mistakes where the endpoint URL is
+	// specified as absolute rather than relative. It only makes a difference
+	// when there's a non-empty base URL path. So, use that. See issue #752.
+	apiHandler := http.NewServeMux()
+
+	server = httptest.NewServer(apiHandler)
+}
+
+// teardown closes the test HTTP server.
+func teardown() {
+	server.Close()
+}
+
+func TestGetJIRAClient_IssueNotExists(t *testing.T) {
+	setup()
+	defer teardown()
+
+	c, err := getJIRAClient(server.URL, "", "")
+	if err != nil {
+		t.Errorf("Failed to create a JIRA client: %s", err)
+	}
+
+	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+	})
+
+	err = checkIfIssue("WEB-1234", c)
+	if err == nil {
+		t.Error("No error occuered. Expected a 404")
+	}
+}
+
 func TestGetIssuesOutOfMessage(t *testing.T) {
 	dataProvider := []struct {
 		Message string
@@ -16,12 +65,33 @@ func TestGetIssuesOutOfMessage(t *testing.T) {
 		{"[SCC-27] Replace deprecated autoloader strategy PSR-0 with PSR-4", []string{"SCC-27", "PSR-4"}},
 		{"WeB-4711 sys-1234 PRD-5678 remove authentication prod build for now", []string{"WeB-4711", "sys-1234", "PRD-5678"}},
 		{"TASKLESS: Removes duplicated comment code.", nil},
+		{"This is a commit message and we applied the PHP standard PSR-0 to the codebase", nil},
 	}
 
 	for _, data := range dataProvider {
-		res := GetIssuesOutOfMessage(data.Message)
+		res := getIssuesOutOfMessage(data.Message)
 		if reflect.DeepEqual(data.Result, res) == false {
 			t.Errorf("Test failed, expected: '%+v' (%d), got: '%+v' (%d)", data.Result, len(data.Result), res, len(res))
 		}
 	}
 }
+
+func TestGetTextToAnalyze(t *testing.T) {
+	dataProvider := []struct {
+		Message string
+		Stdin   bool
+		Result  string
+	}{
+		{"", false, ""},
+		{"From arguments without stdin", false, "From arguments without stdin"},
+		{"From arguments with stdin", true, "From arguments with stdin"},
+		// TODO: Test stdin
+	}
+
+	for _, data := range dataProvider {
+		text := getTextToAnalyze(data.Message, data.Stdin)
+		if text != data.Result {
+			t.Errorf("Test failed, expected: '%+v', got: '%+v'", data.Result, text)
+		}
+	}
+}
