Das deutsche Python-Forum

Hallo Leute,
ich versuch mich gerade mit PyUnit anzufreunden und wollte als Beispiel eine GUI, die ich mal programmiert habe, Refaktorieren und mit Unittests ausstatten...

Was ich in meinem Programm früher gemacht habe, sieht folgendermaßen aus: Ich weiß jetzt nicht, ob das was ich da gemacht habe die feine englische Art ist, bin hier gerne für Verbesserungsvorschläge offen. Mein Programm hat zumindest Objekte erzeugt und ich hab nur einen Dateinamen übergeben, den Rest hat das Objekt selbst gemacht.

Ich will das gerade umbauen und auf ein IOError testen, wenn ich einen Dateinamen angebe, der nicht existiert, oder eben nicht lesbar ist. Mein Test sieht gerade mal so aus.
Der erste Test geht, weil ich ja einfach nur Teste ob ich den Dateinamen eben speichere und abfragen kann. Aber mein zweiter Test will nicht, und sagt "AttributeError: 'Test' object has no attribute 'checkFileReadable'"

Der neue Code sieht soweit so aus: Der Test würde so oder so fehlschlagen, da ich ja den Fehler mit meinem Try Except abfange, aber selbst wenn ich das Kommentar mit dem open weg mache, dann bekomme ich mein IOError aber der Test ist Failed...

Also zum einen meine Frage, ist das ganze methode in der __init__ aufrufen ein bescheidenes Konzept, oder wie macht man das besser? Initialisiert man seine Objekt und iteriert danach noch mal drüber mein einem readCfgFile() oder sowas?

Würde mich über ein Feedback freuen, danke.

@würmchen: Die Klasse sieht nicht besonders gut aus. Die Namen halten sich nicht an PEP8. Es werden unnötige Abkürzungen und Namenszusätze verwendet. Mindestens `readHeadInfo()` und `readFormInfo()` sind keine Methoden. `readCfgFile()` im Grunde auch nicht wirklich. Der Präfix `read` hat verschiedene Bedeutungen, einmal lesen aus einer Datei und dann parsen von Zeilen. Es werden Attribute ausserhalb der `__init__()` neu eingeführt. Namen von Attributen und Methoden passen nicht zusammen.

Wenn man das laden von der `__init__()` aus macht, dann kann man so ein Objekt gar nicht ohne eine Datei erstellen. Das heisst zum Beispiel für Tests, das man immer eine Datei für die Testdaten benötigt und so ein Objekt nicht einfach nur im RAM erstellen kann. Ich stelle für solche Objekte in der Regel eine Klassenmethode als Alternative zur Verfügung welche Daten lädt und dann ein Objekt damit erstellt.

Für Konfiguration gibt es doch aber schon so viele Lösungen. `ConfigParser` oder `json` aus der Standardbibliothek. Oder `ConfigObj`. Warum hast Du das denn noch einmal neu erfunden?


Bei der zweiten Klasse passt der Methodenname `checkFileReadable()` nicht so richtig zur Methode. Ich würde einen Rückgabewert erwarten. Wenn die Datei geöffnet werden kann, dann wird sie nicht explizit wieder geschlossen. Letztlich ist so eine Prüfung aber auch unsinnig, denn die ist im öffnen und lesen einer Datei ja schon enthalten — dann wird eine Ausnahme ausgelöst. Um diese Ausnahme muss man sich so oder so kümmern, auch wenn die `checkFileReadable()` ergeben hat das die Datei zum Zeitpunkt des Tests lesbar war. Das heisst ja nicht das sie es beim tatsächlichen lesen immer noch ist.

würmchen hat geschrieben:

Code: Alles auswählen

def testConfigFileInit(self):
        testfile = "test/test.cfg"
        myconffile = uguiconfigfile.ConfigFile(testfile)
        self.assertEqual(myconffile.filename, testfile, "filename isn't saved correctly.")
        self.assertRaises(IOError, uguiconfigfile.ConfigFile.__init__(self, "test"))

Der Test mit `.assertRaises()` ist total abhängig von äußeren Faktoren. Das Dateisystem bzw der aktuelle Pfad, muss hier exakt so beschaffen sein, dass die Erstellung wunschgemäß fehlschlägt. Das mag theoretisch auch auf anderen Systemen klappen, ist aber nicht besonders elegant. Ich bin selber nicht allzu erfahren in Sachen Unittest, würde aber vom Gefühl her die benötigten Dateien in einem isolierten Verzeichnis extra für den Test anlegen oder ich würde Tests, die Einfluss auf Bereiche außerhalb von Python nehmen, einfach weglassen. Bedenke auch, dass du im Prinzip nur eine Art Backend (`open()`) auf Korrektheit testest. Dass diese Builtin-Funktion erwartungsgemäß arbeitet, kann man aber auch so schon voraussetzen.

Hui, unglaublich wie viele Leute hier nachts noch rumwerkeln Danke

@snafu, ich habe extra eine Config Datei für diesen Zweck angelegt, hab wie gesagt gerade erst angefangen Mit Unittests zu spielen und weiß teilweise noch nicht was sinnvoll ist zu testen. Eigentlich völlig richtig, eine Inbuild Funktion zu testen ist doch recht überflüssig.

Mein Gedanke war, "Als nächstes will ich die Datei lesen, also benötige ich einen Test ob die Datei lesbar ist". Ich weiß im Moment noch nicht, wie klein ich die Schritte wählen sollte, also mit meinen Tests, und welche Test wirklich sinnvoll sind.


@BlackJack, wenn es nach mir ginge hätte ich keinen neuen config Parser geschrieben, aber meine Arbeitsgruppe wollte ein besonders einfaches Format haben und es bekommt leider jeder eine Krise, wenn er ein = oder eine () {} [] sieht. Meine Datei ist so aufgebaut, dass jede Zeile mit einem Codewort (es gibt nur 6 Stück) anfängt und der Rest der Zeile einfach diesem zugewiesen wird. Sowas hab ich leider nicht gefunden.

Weiterhin "versuche" ich mich bei meinem neuen Versuch an PEP8 zu halten. Kannte das damals noch nicht so richtig und es ist mit der Grund, warum ich das ganze noch mal überarbeite.

Was ich nicht ganz nachvollziehen kann, wie kommst Du auf readHeadInfo() und readFormInfo() keine Methoden sind? Weil sie keinen Rückgabewert liefern? Ansonsten kann ich alle Hinweise zur Namensgebung voll und ganz nachvollziehen. Ich glaub das braucht einfach viel Zeit, bis man das richtig einschätzen kann. Und wie gesagt, ich will es jetzt ja auch "richtiger" machen

Aber wäre dann der bessere Ansatz in der __init__ nur meinen Dateinamen zu setzen und dann nach der Initialisierung eine Methode aufzurufen die das ganze setUp macht? Also zwei Schritte? Wenn ich das noch weiter aufteile kann ich die Hinweise von @snafu einbauen, dass ich zum Beispiel anstatt ein echtes File zu übergeben eben eine Variable setzen könnte, in der Quasi das eingelesene File steht?
Macht das Sinn was ich hier quatsche? Ist noch früh

Also Quasi zum Initialisieren dann sowas und nicht mehr alles direkt beim Initialisieren...

Ich übergäbe der __init__-Methode einen String, welcher dann geparst wird. So ist man frei, was dessen Herkunft (Datei, hart kodierter String, Netzwerk, etc.) angeht.

Das Lesen aus einer Datei maximal als 'classmethod' anbieten, welche ein fertiges ConfigObject zurück gibt.

Und ja, ich nähme auch json ...

Grüße ... bwbg

würmchen hat geschrieben:Mein Gedanke war, "Als nächstes will ich die Datei lesen, also benötige ich einen Test ob die Datei lesbar ist".

Das Testen auf Lesbarkeit einer Datei ist eher etwas für den tatsächlichen Betrieb und gehört IMHO nicht in irgendwelche Unittests. Die Tests beziehen sich ja nur auf die Funktionaliät deines Programms. Und diese ist grundsätzlich auch dann gegeben, wenn das Betriebssystem wegen einem unerlaubten Dateizugriff rummeckert. Denn dein Programm hat mit seinem `open()`-Aufruf ja erstmal nichts falsch gemacht. Außer natürlich, wenn die Pfadangabe falsch war und die Datei gar nicht erst gefunden wurde. Wobei man hier wieder zwischen Benutzereingabe und fest im Programm codiertem Pfad unterscheiden sollte, denn für falsche Nutzereingaben kann dein Programm ja nichts.

Du könntest höchstens testen, ob dein Programm dann eine spezielle Exception wirft, falls du die übliche Exception von Python irgendwie abfängst. Das ist dann aber kein Testen auf Lesbarkeit, sondern eher ein Testen darauf, ob ein Fehlerfall wunschgemäß signalisiert wird. Du würdest dann also auf die Exception hin testen, aber nicht auf die eigentliche Lesbarkeit. Vielleicht meintest du das ja auch so. Ich empfehle jedenfalls, den Test so zu benennen, dass möglichst keine Missverständnisse beim Leser auftreten.

@würmchen: Beim Test für so ein Config-Objekt sind eher so Sachen interessant wie was passiert wenn die Datei nicht das erwartete Format hat. Wenn Schlüssel gar nicht vorkommen, oder doppelt, und was passiert wenn man auf Informationen zugreifen möchte die es nicht gibt.

Ich persönlich schreibe in der Regel zu wenig Tests. Meistens kommen die bei mir hinzu wenn es Fehler im Programm gab. Dann schreibe ich einen Test der den Fehler auslöst und an dem man dann sehen kann ob/das der Fehler hinterher auch tatsächlich behoben ist.

Warum musst Du denn bei einem so einfachen Format zweimal durch die Zeilen der Datei iterieren?

Die beiden `read*Info()`-Dateien liefern doch einen Rückgabewert. Die Frage ist ob sie das Objekt überhaupt benutzen. Wenn nicht, warum sind sie dann in der Klasse? `readCfgFile()` ist jedenfalls IMHO nur so ganz knapp eine Methode. Wenn die beiden `read*Info()`\s keine echten Methoden sind, dann könnte man auch `readCfgFile()` locker als Funktion ausdrücken. Den Dateinamen könnte man auch als Argument übergeben.

Nochmal zur Namensgebung: Man könnte aus allen Methoden und Attributen das Kürzel `cfg` rauslassen. Eine Klasse ist ja ein Namensraum, damit sagt `CfgFile.readCfgFile()` letztlich nicht mehr aus als `CfgFile.read()`.

Ich würde das wie gesagt in zwei Schritte aufteilen, eigentlich sogar drei mit Blick auf Tests, aber in umgekehrter Reihenfolge, so dass der Benutzer immer nur einen Schritt ausführen muss. Also der Benutzer muss nicht immer ein Objekt erstellen und dann die Methode zum Laden aufrufen, sondern er ruft eine Klassenmethode zum Laden auf und die gibt ihm dann ein fertig geladenes Konfigurationsobjekt zurück. Die `__init__()` bekommt dazu alles was man für ein komplettes Objekt benötigt übergeben. So ist dann auch sichergestellt, dass nach der `__init__()` das Objekt in einem „fertigen” und komplett benutzbaren Zustand ist. Das könnte ungefähr so aussehen (ungetestet): Hier hat man nun drei Möglichkeiten mit jeweils *einem* Aufruf ein `Config`-Objekt zu erstellen. Entweder direkt mit den passenden Daten, oder aus einem iterierbaren Objekt mit Zeilen, oder einem Dateinamen.

@snafu, ich wollte eigentlich auf Exception testen, aber mir war nicht klar, wie ich das bei meiner "Designidee" realisieren sollte, da ich ja schon alles bei der Erstellung mache... Deswegen war das dann mehr so eine Designfrage/PyUnitfrage

@BlackJack wow, das sieht mal extrem elegant aus, vor allem in Bezug auf das Testen! Ich kannte die @classmethod Option nicht, bzw wie ich eben festgestellt hab ich kenne fast keine function decorator... Ok, wieder was gelernt. Werde das auf jeden Fall mal so versuchen.

Ja, mit der Namensgebung und dem eigenen Namespace hast Du mehr als recht. Aber ich glaub das ist einfach Erfahrung die man auch sammeln muss. Ich hab letztens das Buch Clean Code gelesen und würde am liebsten durch all meine Programme durchgehen. Das Problem ist nur immer, man hat es gelesen, es ist alles plausibel, aber es dann in der Praxis anzuwenden geht nicht von heute auf morgen.

Ich danke auf jeden Fall für die Hinweise und die Vorschläge, werde mit den UnitTest mal mehr herum probieren.

Da muss ich sagen ist Eclipse ein echter Segen! Ich bin normalerweise VI verfechter, aber wenn man ständig seinen Code Ausführen will ist das ja Pain in the Ass! PyDev macht es einem unheimlich einfach und gibt bei jedem Speichern ein Feedback wo man steht. Da macht entwickeln Spaß!

@würmchen: Ich würde mich stark wundern wenn man Vim nicht auch dazu bringen kann Code per Tastenkürzel auszuführen und beim speichern einen oder mehrere „linter” (pycheck, pylint, pep8, …) über den Quelltext laufen zu lassen. Ich verwende SublimeText 2 als Editor und da geht das.

Ja, geht bestimmt, aber es ist schon alles viel komfortabler mit Eclipse, auch die Vervollständigung wenn noch keine Funktion oder Methode vorhanden ist. Man kann quasi "fast nur" die TestCase Klasse editieren und wenn man einen Test definiert, über eine Funktion die noch nicht existiert, kann man die sich mit Strg+1 einfach erstellen lassen, Namen der Variablen noch anpassen und gut. Nicht ständig in unterschiedliche Dateien springen etc.

Ich bevorzuge nach wie vor VI um "mal schnell" was zu coden... Aber Pakete, die aus mehreren Dateien bestehen, ist es doch echt ein Segen...

Mein VI speichert übrigens die Datei und führt sie aus beim drücken auf F5... vorausgesetzt es ist eine .py Datei...