Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace main() with unit test to make Travis CI work #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Philipp91
Copy link
Collaborator

Initially, I'm just creating this pull request hoping that Travis will eventually complain about a failure in there. Later, I will update it without causing the failure, but keeping the code that catches it.

@Philipp91
Copy link
Collaborator Author

@Philipp91
Copy link
Collaborator Author

@Philipp91
Copy link
Collaborator Author

Now the branch contains the unit test change without the change that makes the decryption fail, so it should be ready for review.

@Philipp91 Philipp91 changed the title TO NOT MERGE: This is making the decryption fail on purpose Replace main() with unit test to make Travis CI work Jul 17, 2018
@Philipp91 Philipp91 requested a review from kmindi July 17, 2018 18:47
Copy link
Collaborator

@kmindi kmindi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Siehe TODO Kommentar, der Test sollte noch die Ausgabe prüfen.
=> Dafür sollte das Programm noch eine Ausgabe erzeugen. Anscheinend kann man das dann ganz gut mit assert_cli (https://mattgathu.github.io/testing-rust-cli-apps/) machen.

Der Key sollte vermutlich nicht in eine Datei geschrieben werden. Das sollte in einem "Key Storage" Beispiel erklärt werden. Also man müsste den Key noch mit einem Passwort schützen/verschlüsseln.

@Philipp91
Copy link
Collaborator Author

Philipp91 commented Jul 18, 2018

Das mit dem Prüfen der Ausgabe wäre ein bisschen umständlich.
Dafür bräuchte man Kommandozeilen-Flags, mit denen man bestimmen kann, welches Codebeispiel ausgeführt werden soll, eine main-Methode die mit einem riesigen switch-Statement die richtige Methode ausführt, und pro Codebeispiel einen Unit-Test der die Binary mit den richtigen Flags aufruft und die Ausgabe kontrolliert. Und, wie du gesagt hast, müsste man die Ausgabe auch erst einmal einfügen. Mir ist aber nicht wirklich klar, was der Mehrwert wäre.

Wenn du eine Assertion haben möchtest, könnte die Methode auch true/false zurückgeben, je nach dem ob es geklappt hat oder nicht, und der Test kann prüfen, dass es true ist. Das wäre dann eine leichtgewichtigere Art des Result<(), Error> Rückgabewerts den ich früher da hatte.

Das mit dem Key stimmt. Im analogen Java-Beispiel (https://www.cryptoexamples.com/java_string_encryption_key_based_symmetric.html) wird der Schlüssel einfach nicht gespeichert, sondern beim Ver- und Entschlüsseln aus dem gleichen Objekt im Speicher ausgelesen. Wäre das hier auch ok?
Es lässt halt für den Nutzer das Problem offen, wie man den Key speichert. Denn jedes Mal neu generieren funktioniert ja eben nicht. Und da besteht vielleicht die Gefahr, dass die Leute ihn dann hartkodieren.

@kmindi
Copy link
Collaborator

kmindi commented Jul 19, 2018

Ok, aber was macht der Unit test sonst? Er führt die Funktion aus.
Kannst du es so wie in diesem Java-Beispiel machen: https://www.cryptoexamples.com/java_string_encryption_password_based_symmetric.html (d.h. es gibt eine Funktion die etwas zurückgibt und eine Main-Methode (damit das Beispiel ausführbar ist, bzw. etwas demonstriert)) entsprechend der Test dazu: https://github.com/cryptoexamples/java-crypto-examples/blob/master/src/test/java/com/cryptoexamples/java/EncryptionInOneMethodTests.java#L40
Also so wie du in deinem zweiten Absatz selber vorschlägst.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants