Zeig mir deinen Code!

Gerrit & Co. – Codereviewprozesse und die Commit Stage
Kommentare

In vielen Open-Source-Projekten hat sich ein sehr formalisierter Reviewprozess für Codeänderungen etabliert. Beigetragen haben dazu GitHub mit der Verbreitung von Pull Requests und Codereviewsysteme wie Gerrit.

Pull Requests unterstützen zunächst einmal manuelle Codereviews. Man kann über den Pull Request diskutieren und alle Erkenntnisse zusammentragen, um den Request zu bewerten. Es gibt kein formales Kriterium, wann ein Pull Request für gut befunden oder abgelehnt wird. Der Prozess ist recht flexibel.

Gerrit ist ein Codereviewwerkzeug, das den Prozess weiter formalisiert. Es kombiniert Feedback vom Continuous-Integration-(CI-)System, das besagt, ob der Code erfolgreich baut, und Einschätzungen von Reviewern zum Code. Beides hat zum Ziel, die Baseline stabil zu halten. Allerdings braucht dieser Prozess Zeit, und die Dauer bis zu einem Feedback über eine Codeänderung verlängert sich durch das manuelle Review. Dieser manuelle Reviewprozess eignet sich gut für bestehende Software mit relativ wenigen Codeänderungen pro Arbeitstag. Während einer Produktentwicklung entstehen aber viele Codeänderungen (neuer und umgebauter Code), sodass viele Teams es als unpraktisch ansehen, jede Änderung zeitnah vor einem Commit in den Master-Branch zu begutachten. Stattdessen definieren agile Teams zum Beispiel, dass ein Codereview zum Abschluss einer User Story notwendig ist (Post-Commit Review). Wenn aber auf diesen Reviewprozess verzichtet wird, steigt durch Continuous Integration auch die Gefahr von „Continuously Broken Builds“.

Wenn ein CI Build nicht erfolgreich ist, so bedeutet dies, dass sich im Hauptentwicklungszweig „schlechter“ Code befindet. Jeder Entwickler, der einen Merge seines lokalen Entwicklungszweigs mit diesem Code durchführt, holt sich Code in seinen Arbeitsbereich, der verhindert, dass er weiter erfolgreich die Applikation bauen kann. Er muss also seine Arbeit unterbrechen. Das Mindeste, was nun auf ihn zukommt, ist, diesen Merge zurückzurollen. Eventuell stellt der Programmierer das Problem auch erst nach einer umfangreicheren Merge-Sitzung fest. Wenn das häufig vorkommt, wird viel Zeit verschwendet, was auf Seiten der Entwickler zu Frustrationen führt.

Automatisches Codereview

Wenn wir auf das manuelle Codereview verzichten, dann können wir immer noch einen automatischen Review durchführen. Ein automatischer Review kann alles enthalten, was wir für notwendig erachten, um den Patch zu beurteilen: alle Arten von Tests, die wir für angebracht halten, insbesondere Unit und Integrationstests, aber natürlich auch statische Sourcecode-Analyse.

Aber auf dem Entwicklerrechner können höchstens die Unit Tests durchgeführt werden, denn der Build muss schnell ablaufen. Darüber, was „schnell“ heißt, gibt es unterschiedliche Meinungen. Wenn man davon ausgeht, dass der Build zweimal laufen muss (lokal und auf dem Integrationsserver), bevor man entsprechend den oben aufgestellten Regeln weiter entwickeln darf, dann sollte der gesamte Prozess nicht länger als fünf bis zehn Minuten dauern, also so lange, wie es dauert, sich eine neue Tasse Kaffee zu holen und kurz ein paar Worte mit den Kollegen oder dem Pairing-Partner zu wechseln. Das heißt, ein einzelner Build darf nicht mehr als fünf Minuten Zeit in Anspruch nehmen. Das geht aber nur, wenn die große Mehrzahl der Tests (schätzungsweise über 80 bis 90 Prozent) echte Unit Tests sind. Und je mehr Tests es insgesamt gibt, umso höher muss die Quote der echten Unit Tests sein. Dies entspricht einem Eisbergmodell, bei dem auch nur ca. 10 Prozent sichtbar über dem Wasser sind. Die reinen Unit Tests selbst müssen dann in deutlich unter fünf Minuten durchlaufen werden können. Hier liegt meiner Erfahrung nach die Toleranzgrenze bei circa zwei Minuten.

Auch wenn ich ein Anhänger des Eisbergmodells bin und dieses in die Projekte trage, kann ich nicht die Augen vor der Realität verschließen. Bei größeren Projekten nimmt die Zahl der Integrationstests zu. Als Integrationstests zählen dabei alle Tests, bei denen die Dauer für das Herstellen der Testumgebung signifikant ist. Meiner Ansicht nach liegt die Grenze bei ca. zwei Sekunden. Tests brauchen schnell länger, wenn sie mittels O/R Mapper Teile des Datenbankzugriffs testen oder mittels ShrinkWrap/Arquillian ein Archiv bauen, das dann auf einem Applikationsserver ausgeführt wird. Teams fordern häufig, dass neben Unit Tests auch zeitlich aufwändigere Tests wie Teile der Integrationstests durchgeführt werden, um Code als produktionsreif und damit als geeignet für die Aufnahme in den Hauptentwicklungszweig zu betrachten. Die Ausführung dieser Tests auf dem Entwicklungsrechner blockiert jedoch den Entwickler in seinem Arbeitsfluss.

Aber warum sollte ein Entwickler seinen Code überhaupt lokal bauen? In Zeiten, in denen Cloud Computing in Firmen und Projekte Einzug hält, sind häufig genügend freie Serverressourcen vorhanden, um die Builds aller Entwickler problemlos und häufig sogar schneller durchzuführen als auf dem lokalen Entwicklerrechner. Moderne CI-Systeme unterstützen dabei die Nutzung von Slaves oder Build-Agenten, die auf virtuellen Maschinen in der Cloud laufen können. Dabei können auch seltener genutzte Systeme als Build-Agenten genutzt werden.

Und wenn wir schon Entwickler sind, wollen wir das Problem der Broken Builds im Hauptentwicklungszweig nicht durch organisatorische Regelungen beheben, sondern technisch durch einen mehrstufigen CI Build. Das CI-System soll schließlich für den Entwickler arbeiten und nicht umgekehrt. Ein Entwickler sollte zudem seinen Code zentral einchecken können, um seine Arbeit zu sichern, ohne dass andere Teammitglieder durch unfertigen Code gestört werden. Um dies zu realisieren, bräuchten wir also einen privaten Bereich im Versionskontrollsystem, in dem jeder Entwickler seine Arbeit sichern kann, und Jobs auf dem CI-Server, die seine persönlichen Builds ausführen.

Wenn wir weiterhin von dem Ziel ausgehen, eigentlich auf dem Hauptentwicklungszweig zu arbeiten, dann sollte das CI-System noch mehr leisten: einen Merge des persönlichen Codes mit dem Hauptentwicklungszweig (solange es keine Konflikte gibt), einen Build und nach erfolgreichem Build einen Commit des zusammengeführten und erfolgreich gebauten Codes in den Hauptentwicklungszweig – also genau das, was bei einem Reviewprozess getan würde. So stellen wir sicher, dass sich ein persönlicher Zweig nicht weit vom Hauptentwicklungszweig entfernt, und Änderungen mit diesem kompatibel sind. Das Vorgehen hat einige Vorteile:

  • Jeder Entwickler kann durch persönliche Builds seinen mit dem Hauptentwicklungszweig zusammengeführten Code zentral bauen und testen lassen.
  • Tests auf dem CI-System können umfangreicher sein als lokale Tests, und der Entwickler kann zwischenzeitlich weiterarbeiten.
  • Es findet ein regelmäßiger, kontrollierter Abgleich mit dem Hauptentwicklungszweig statt.
  • Der Hauptentwicklungszweig wird stabiler.

Dieses Muster ist auch bekannt als Personal Build, Preflight Builds oder auch Pre-Tested Commits. Hier verwenden verschiedene CI-Server-Hersteller unterschiedliche Namen. Jenkins als einer der verbreitetsten CI-Server bietet dieses Feature aber nicht direkt an.

Umsetzung mit Jenkins

In den Projekten, in denen mein Team diese Lösung entwickelte, sollte nur eine automatische Codeprüfung stattfinden. Gerrit sollte nicht eingesetzt werden. Also war eine Lösung gefragt, die mit Jenkins als CI-Server und mit Git als Versionskontrollsystem auskam. Wie sich im Nachhinein herausstellte, finden sich wesentliche Aspekte (z. B. Namenskonventionen) bei Gerrit ähnlich wieder.

Es stellte sich also die Frage, wie man mittels Jenkins und Git Private Builds abbilden kann. Einige CI-Systeme bieten dieses Feature gut dokumentiert an, Jenkins stellt aber keine direkte Unterstützung zur Verfügung. Und wahrscheinlich ist das auch der Grund, weshalb viele Teams die Möglichkeit eines Personal Builds nicht nutzen. Die Umsetzung beschreibe ich im Folgenden. Die Entwicklung erfolgte auf einem Hauptentwicklungszweig development. Zusätzlich verfügte jeder Entwickler über einen privaten Entwicklungszweig, der einer Namenskonvention entsprach. Für die Entwicklerin Alice hieß dieser Zweig alice/for-development. Auf dem Jenkins-Server wurden zwei Jobs eingerichtet:

  • Der erste Job startet die Build-Pipeline für den Hauptentwicklungszweig development.
  • Der zweite Job startet die Build-Pipeline für den Personal Build **/for-development.

Das Git-Plug-in erlaubt es, über einen Wildcard-Ausdruck alle Zweige zu überwachen, die einem bestimmten Muster entsprechen. Bei einer Änderung in einem privaten Entwicklungszweig checkt dieser Job den aktuellen Stand des Integrationszweigs development aus, führt den geänderten for-development-Zweig mit development zusammen und baut diesen Stand. Der Build arbeitet also auf dem Hauptentwicklungszweig mit den aktuellen Änderungen des Entwicklers. Waren Merge, Build und Test erfolgreich, wird dieser neue Stand von Jenkins ins zentrale Repository gepusht (Abb. 1).

Aus Entwicklersicht stellt sich der Workflow wie in Abbildung 2 beschrieben dar. Jeder Entwickler holt sich die Änderungen über den Hauptentwicklungszweig development in seinen privaten Entwicklungszweig. Es erfolgt niemals ein Merge mit dem Zweig eines anderen Entwicklers, sondern nur mit dem Integrationszweig development (außer man weiß, was man tut). Gleichzeitig pusht ein Entwickler nie in den Integrationszweig.

Abb. 1: Workflow während des Builds

Abb. 1: Workflow während des Builds

Abb. 2 Workflow aus Entwicklersicht

Abb. 2 Workflow aus Entwicklersicht

Damit dieser automatische Reviewprozess nicht unterlaufen werden kann, wurde nur dem CI-Nutzer (Jenkins) erlaubt, auf den Hauptentwicklungszweig development zu pushen. Mit Repository-Management-Systemen wie GitLab oder Stash lässt sich das leicht über Berechtigungen steuern. Dies schützt den Hauptentwicklungszweig nochmals vor unbeabsichtigten Änderungen.

Git-Plug-in-Konfiguration

Die Konfiguration von Jenkins ist relativ einfach. Es reicht aus, das Git-Plug-in wie in Abbildung 3 zu konfigurieren.

Abb. 3: Git-Plug-in-Konfiguration

Abb. 3: Git-Plug-in-Konfiguration

Der erste Teil der Konfiguration weist das Git-Plug-in an, einen Build auszuführen, wenn ein Zweig mit der Namenskonvention **/for-development gepusht wird. Der zweite Teil bewirkt, dass das Git-Plug-in den Integrations-Branch development lokal auscheckt und den geänderten Zweig dort hinein integriert. Sollte dies schon schiefgehen, wird der Build abgebrochen. Wenn der Build erfolgreich durchläuft, sorgt der dritte Teil dafür, dass der geänderte Entwicklungszweig von Jenkins ans Sourcecode Repository übertragen wird.

Erfahrungen

Seit 2012 haben wir den vorgestellten Arbeitsablauf in verschiedenen Projekten eingesetzt. Gerade in einem Neuentwicklungsprojekt, bei dem viel Code in kurzer Zeit produziert wurde und es noch häufig zu Codeumorganisationen kam, verringerte der vorgestellte Workflow schnell die Anzahl der Frustrationsmomente der Entwickler. Sie konnten nun sicher sein, dass ihr privater Entwicklungszweig auch nach einem Merge mit dem Hauptentwicklungszweig noch erfolgreich bauen würde.

Über die Nutzung von Pre-Tested Commits kann also das Vertrauen in den Integrationszweig erhöht werden, indem vor einem Commit in diesen Zweig eine Reihe automatischer Tests (etwa Unit Tests oder auch statische Codeanalyse) durchgeführt werden. Es ist Aufgabe des Entwicklers, dafür zu sorgen, dass sein Code den automatischen Reviewprozess erfolgreich durchläuft und somit seine Änderungen Aufnahme in den Hauptentwicklungszweig finden. Die Entwickler müssen aber die Mitteilungen des CI-Systems beachten. Meldet das CI-System Fehler, dann ist Ihr Code nicht erfolgreich in den Hauptentwicklungszweig integriert worden.

Nicht offensichtliche Details

Jede Codeänderung eines Entwicklers soll eine Instanz der Build-Pipeline starten, damit diese Änderung isoliert überprüft wird. Daher sollte nicht ein Polling des Sourcecode-Systems (etwa alle fünf Minuten) stattfinden, sondern das Sourcecode Repository sollte direkt bei einem Commit einen Build auslösen. Dies ist recht einfach möglich, wenn man die Möglichkeit von Git Hooks oder Web Hooks von Git, GitLab oder Stash nutzt.

Selbst wenn man die einzelnen Codeänderungen isoliert baut, kann es aufgrund der Dauer einer Build-Pipeline dazu kommen, dass die Integration der Codeänderungen in den Hauptentwicklungszweig am Ende der Pipeline nicht funktioniert. Das tritt auf, wenn der CI-Server einen anderen privaten Build in den Hauptentwicklungszweig pusht, während der eigene Private Build noch läuft. Ein typisches Problem der Nebenläufigkeit, wenn mehrere Instanzen der Build-Pipeline parallel ausgeführt werden können.

In unserem siebenköpfigen Team ist dieser Fall jedoch so selten aufgetreten, dass sich niemand sicher erinnern konnte, ob er überhaupt vorgekommen ist. Sollte das Problem tatsächlich einmal auftreten, kann es durch einen neu angestoßenen Build recht einfach behoben werden. Die Zeiten, zu denen unterschiedliche Entwickler einen Private Build anstoßen, verteilen sich erfahrungsgemäß recht gut über den Tag. Anders sieht es allerdings aus, wenn sehr viele Entwickler auf dem Hauptentwicklungszweig arbeiten. Dann aber wäre die Codeorganisation, die Repository- und ggf. die Branching-Strategie ohnehin zu überdenken.

Wie in der Einleitung schon angedeutet, ist das Ziel eines automatischen oder manuellen Codereviews unter anderem, die Stabilität des Integrationszweigs sicherzustellen. Je aufwändiger der Codereview gestaltet ist, desto größer ist die Dauer zwischen Commit und Feedback. Es müssen also unterschiedliche Anforderungen ausbalanciert werden: zum einen die Sicherheit, dass der Hauptentwicklungszweig immer releasefähig ist. Zum anderen die Bereitstellung von schnellem Feedback sowie die Verteilung von Bugfixes und neuen Features an die Kollegen über den Hauptentwicklungszweig. Wichtig ist hierbei, dass das Team definiert, welche Kriterien es braucht, um die Integration eines privaten Zweigs in den Hauptentwicklungszweig zuzulassen. Über diese Kriterien sollte das Team regelmäßig reflektieren. In einem Projekt startete ein Team zum Beispiel mit der Voraussetzung, dass alle Unit und Integrationstests laufen müssen. Das Team hatte zu dieser Zeit ein hohes Sicherheitsbedürfnis. Im Laufe der Zeit wurde das Projekt stabiler, und dem Team reichten Unit Tests und wenige kritische Integrationstests. Dabei kam es dann durchaus vor, dass der Build des Hauptentwicklungszweigs einmal innerhalb von zwei Wochen kaputtging. Da mehrmals täglich ein vollständiger Build mit allen Tests lief, reichte es dem Team aus, dies mit maximal vier Stunden Verspätung zu erfahren. Die Fehler waren dann immer noch schnell zu lokalisieren und zu beheben. Das Bedürfnis nach schnellerer Bereitstellung von Bugfixes und Features für das ganze Team überwog hier gegenüber dem Sicherheitsbedürfnis.

Kritische Betrachtungen

Die Nutzung von Personal Builds kann dazu führen, dass man sich nicht oder zu spät um die Ursache für lang laufende Builds kümmert. Da man länger mit solchen problematischen Builds leben kann, werden Ursachen erst spät oder gar nicht adressiert, wie

  • zu viele Integrationstests anstelle von Unit-Tests
  • zu viele End-to-End-Tests anstelle von Integrationstests
  • ein CI-Job, der zu viele Aufgaben hat, statt einer Build-Pipeline mit kleinen, fokussierten Jobs

Daher warne ich davor, Personal Builds als Entschuldigung zu nutzen, um solche Builds zu tolerieren, anstatt über Code-, Test- und Build-Organisation nachzudenken.

Alternativen

Auch für diejenigen, die nicht Git und Jenkins einsetzen, gibt es einige Alternativen. Zum einen gibt es kommerzielle CI-Server, die diesen Workflow direkt unterstützen (z. B. JetBrains TeamCity oder Atlassian Bamboo), zum anderen gibt es auch die Möglichkeit, diesen Workflow mit Subversion in Jenkins umzusetzen. Hier nur eine kleine Auswahl von alternativen Umsetzungsmöglichkeiten:

  • Atlassian Bamboo: Bamboo unterstützt das gezeigte Vorgehen unter dem Namen Gatekeeper. Das dort beschriebene Branch Updater Pattern ist ebenfalls mit dem Git-Plug-in umsetzbar.
  • TeamCity Pre-Tested Commit: Dies war unser eigentlicher Motivator für die Umsetzung auf Jenkins. Pre-Tested Commit gibt es in TeamCity schon lange. Da aber in der Zwischenzeit unsere Kunden überwiegend Jenkins einsetzen, suchten wir nach Alternativen.
  • Jenkins Subversion Merge Plugin: Erlaubt die einfache Erstellung von privaten Feature-Branches und CI-Jobs und kann auch so konfiguriert werden, dass ein automatisches Rebase und eine Integration wie beschrieben stattfindet. Zusammen mit Jenkins Autojobs braucht man auch keine Jobs manuell zu kopieren und zu löschen.

Aufmacherbild: website development – programmer writing html code von Shutterstock / Urheberrecht: ronstik‎

Unsere Redaktion empfiehlt:

Relevante Beiträge

Meinungen zu diesem Beitrag

X
- Gib Deinen Standort ein -
- or -