Fix rebase when changeIDs are missing (#43)

Context
---

When git-review is run a first time when the git hook is not installed,
maiao wil do 2 things:

1. suggest to install the git hook
2. perform a rebase so all commits are updated with a changeID

When establishing the list of commits, maiao walks through the log,
from the most recent to the oldest, starting from the repo HEAD
and ending with the remote HEAD.

Those commits are then added to the changes in the same order so they
can be used in the rebase TODO list in the relevant order.

Problem
---

The technique used to inject the changes differs whether there is a
changeID or not in the commit message.

If a changeID is present, the commit is added at the beginning of the
change list, ensuring an order from oldest to newest.
If now, the change ID is absent, the commit is added at the end of the
list, making an order from newest to oldest.

Concretely, if the local changes are:

* $sha1 A commit without changeID 1
* $sha2 A commit with a changeID 1
* $sha3 A commit without changeID 2
* $sha4 A commit with a changeID 2

They will bet rebased as following, which may cause rebase conflicts on
the way.

* $sha1 A commit without changeID 1
* $sha3 A commit without changeID 2
* $sha2 A commit with a changeID 1
* $sha4 A commit with a changeID 2

Goal
---

Fix this situation

Change-Id: Id8ee8c9a86ba7081bddc123a31adbca382445582

Co-authored-by: Thibault Jamet <thibault.jamet@adevinta.com>

pkg/maiao/review.go

@@ -395,11 +395,11 @@ func extractChanges(ctx context.Context, repo lgit.Repository, base, head plumbi
 			}
 			changeID, ok := message.GetChangeID()
 			if !ok {
-				changes = append(changes, &change{
+				changes = append([]*change{{
 					commits: changeCommits,
 					head:    changeCommits[len(changeCommits)-1],
 					message: message,
-				})
+				}}, changes...)
 
 			} else {
 				changes = append([]*change{{

pkg/maiao/review_test.go

@@ -4,13 +4,18 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"fmt"
 	"io"
 	"io/ioutil"
+	"os"
+	"os/exec"
 	"strings"
 	"testing"
 
 	"github.com/adevinta/maiao/pkg/api"
 	"github.com/adevinta/maiao/pkg/log"
+	"github.com/adevinta/maiao/pkg/system"
+	"github.com/spf13/afero"
 
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/config"
@@ -323,6 +328,75 @@ func TestRemoveMergedChangeIDs(t *testing.T) {
 	)
 }
 
+func gitCommand(t *testing.T, path string, args ...string) string {
+	t.Helper()
+	cmd := exec.Command("git", args...)
+	cmd.Dir = path
+	out, err := cmd.CombinedOutput()
+	require.NoError(t, err)
+	return strings.TrimSpace(string(out))
+}
+
+func TestExtractChanges(t *testing.T) {
+	d, err := os.MkdirTemp("", t.Name())
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		os.RemoveAll(d)
+	})
+	fs := afero.NewBasePathFs(afero.NewOsFs(), d)
+
+	gitCommand(t, d, "init")
+	gitCommand(t, d, "config", "user.email", "john.doe@example.com")
+	gitCommand(t, d, "config", "user.name", "John Doe")
+	gitCommand(t, d, "commit", "--allow-empty", "-m", "Initial commit")
+	initialCommit := gitCommand(t, d, "rev-parse", "HEAD")
+	system.EnsureTestFileContent(t, fs, "code.txt", `
+	func removeMergedChangeIDs(changes []*change, knownChangeIDs map[string]struct{}) []*change {
+		filtered := []*change{}
+		for _, change := range changes {
+			if _, ok := knownChangeIDs[change.changeID]; !ok {
+				filtered = append(filtered, change)
+			}
+		}
+		return filtered
+	}`)
+	gitCommand(t, d, "add", "code.txt")
+	gitCommand(t, d, "commit", "-m", "Code number one")
+	system.EnsureTestFileContent(t, fs, "code.txt", `
+	func removeMergedChangeIDs(changes []*change, knownChangeIDs map[string]struct{}) []*change {
+		filtered := []*change{}
+		for _, change := range changes {
+		}
+		return filtered
+	}`)
+	gitCommand(t, d, "add", "code.txt")
+	gitCommand(t, d, "commit", "-m", "Code number two")
+	system.EnsureTestFileContent(t, fs, "code.txt", `
+	func removeMergedChangeIDs(changes []*change, knownChangeIDs map[string]struct{}) []*change {
+		filtered := []*change{}
+		for _, change := range changes {
+			fmt.Println(change)
+		}
+		return filtered
+	}`)
+	gitCommand(t, d, "add", "code.txt")
+	gitCommand(t, d, "commit", "-m", "Code number three", "-m", "ChangeId: I1234")
+	fmt.Println(d)
+
+	repo, err := git.PlainOpen(d)
+	require.NoError(t, err)
+	head, err := repo.Head()
+	require.NoError(t, err)
+	b, err := repo.ResolveRevision(plumbing.Revision(initialCommit))
+	require.NoError(t, err)
+	changes, err := extractChanges(context.Background(), repo, *b, head.Hash())
+	require.NoError(t, err)
+	require.Len(t, changes, 3)
+	assert.Equal(t, "Code number one", strings.TrimSpace(changes[0].commits[0].Message))
+	assert.Equal(t, "Code number two", strings.TrimSpace(changes[1].commits[0].Message))
+	assert.Equal(t, "Code number three\n\nChangeId: I1234", strings.TrimSpace(changes[2].commits[0].Message))
+}
+
 func TestNeedReview(t *testing.T) {
 	storage := memory.NewStorage()
 	rootParent := "bdc945b1bc57b3938f7223c7adb8bc2db58b838f"