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

Improve golden file error messages #207

Merged
merged 9 commits into from
Nov 18, 2024
Merged

Conversation

adombeck
Copy link
Contributor

@adombeck adombeck commented Nov 12, 2024

UDENG-5195

This changes the error message from:

Error:      	Not equal: 
expected: map[string]string{"button":"Request new login code", "code":"user_code", "content":"https://verification_uri.com", "foo":"bar", "label":"Scan the QR code or access \"https://verification_uri.com\" and use the provided login code", "type":"qrcode", "wait":"true"}
actual  : map[string]string{"button":"Request new login code", "code":"user_code", "content":"https://verification_uri.com", "label":"Scan the QR code or access \"https://verification_uri.com\" and use the provided login code", "type":"qrcode", "wait":"true"}

Diff:
--- Expected
+++ Actual
@@ -1,2 +1,2 @@
-(map[string]string) (len=7) {
+(map[string]string) (len=6) {
  (string) (len=6) "button": (string) (len=22) "Request new login code",
@@ -4,3 +4,2 @@
  (string) (len=7) "content": (string) (len=28) "https://verification_uri.com",
- (string) (len=3) "foo": (string) (len=3) "bar",
  (string) (len=5) "label": (string) (len=89) "Scan the QR code or access \"https://verification_uri.com\" and use the provided login code",
Test:       	TestSelectAuthenticationMode/Successfully_select_device_auth_qr
Messages:   	SelectAuthenticationMode should have returned the expected layout

to

Error:      	Golden file content mismatch

Expected (golden):
--------------------------------------------------
button: Request new login code
code: user_code
content: https://verification_uri.com
label: Scan the QR code or access "https://verification_uri.com" and use the provided login code
type: qrcode
wait: "true"
foo: bar
--------------------------------------------------

Actual: 
--------------------------------------------------
button: Request new login code
code: user_code
content: https://verification_uri.com
label: Scan the QR code or access "https://verification_uri.com" and use the provided login code
type: qrcode
wait: "true"
--------------------------------------------------

Diff:
--- Expected (golden)
+++ Actual
@@ -4,5 +4,4 @@
 label: Scan the QR code or access "https://verification_uri.com" and use the provided login code
 type: qrcode
 wait: "true"
-foo: bar
 
Test:       	TestSelectAuthenticationMode/Successfully_select_device_auth_qr
Messages:   	Golden file: testdata/TestSelectAuthenticationMode/golden/Successfully_select_device_auth_qr

If delta is installed on the system, the diff is also colorized:

image

@adombeck adombeck force-pushed the improve-golden-file-errors branch 3 times, most recently from 94c482a to e3fe65e Compare November 12, 2024 11:03
@adombeck adombeck marked this pull request as ready for review November 12, 2024 11:04
@adombeck adombeck requested a review from a team as a code owner November 12, 2024 11:04
@adombeck adombeck force-pushed the improve-golden-file-errors branch 2 times, most recently from 73e60d9 to efe4aa8 Compare November 12, 2024 20:27
The function does not do any dconf database filtering
* Remove the "Filtering" part from the name because the function
  doesn't do any filtering
* Mention that it can also update the golden files
We don't need to pass it, the function can access the package variable
instead.
Move function calls out of require.NoError for improved readability.
Print the content in multiple lines instead of a single line.
Use delta to colorize the diff of mismatching golden files if delta is
found in the PATH.
@adombeck
Copy link
Contributor Author

According to ubuntu/authd#583 (comment), there was a discussion before about changing the Ubuntu version used in our GitHub workflows from jammy to noble, and it was decided that we want to stay on jammy for some reason. If that is correct, I'll have to revert 79afbac, and then we won't have the nice output from delta in our test output. I could live with that, but I wonder why we want to stay on jammy.

@adombeck adombeck merged commit bd71c2b into main Nov 18, 2024
4 checks passed
@adombeck adombeck deleted the improve-golden-file-errors branch November 18, 2024 21:41
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.

3 participants