Skip to content

Reverse "want" and "got" in test failures - #279

Merged
mbrt merged 2 commits into
mbrt:masterfrom
lutzky:reverse-test-error
Aug 11, 2022
Merged

mbrt merged 2 commits into
mbrt:masterfrom
lutzky:reverse-test-error

Conversation

@lutzky

@lutzky lutzky commented Aug 10, 2022

Copy link
Copy Markdown
Contributor

Suppose we have a rule configured to label messages with oops-bad-label, and a test that checks they're labeled with yep-correct-label. So before this change you would get:

--- want
+++ got
...
  "labels": [
-   "oops-bad-label"
+   "yep-correct-label"
  ]

To make this match the usual logic of automated testing (especially Go's "want" and "got"), we would like the "want" section to be "what the test describes should happen". Therefore, with this change, we get this output:

--- want
+++ got
...
  "labels": [
-   "yep-correct-label"
+   "oops-bad-label"
  ]

@mbrt

mbrt commented Aug 10, 2022

Copy link
Copy Markdown
Owner

Uhm, you're right! I never realized the test was the opposite.

@lutzky

lutzky commented Aug 10, 2022 •

Copy link
Copy Markdown
Contributor Author

The failure (seen here) is unrelated to this commit... but it prevents tests from running. Would you like a separate PR to fix that?

@mbrt

mbrt commented Aug 11, 2022

Copy link
Copy Markdown
Owner

Ah yes, please. If you have a minute this would be helpful.

lutzky added 2 commits August 11, 2022 08:57
Suppose we have a rule configured to label messages with
"oops-bad-label", and a test that checks they're labeled with
"yep-correct-label". So before this change you would get:

    --- want
    +++ got
    ...
      "labels": [
    -   "oops-bad-label"
    +   "yep-correct-label"
      ]

To make this match the usual logic of automated testing (especially Go's
"want" and "got"), we would like the "want" section to be "what the test
describes should happen". Therefore, with this change, we get this
output:

    --- want
    +++ got
    ...
      "labels": [
    -   "yep-correct-label"
    +   "oops-bad-label"
      ]
@lutzky
lutzky force-pushed the reverse-test-error branch from d22aea5 to 83f71f5 Compare August 11, 2022 08:57
@lutzky

lutzky commented Aug 11, 2022

Copy link
Copy Markdown
Contributor Author

I've done it as a separate commit within this PR, LMK if you want it in a separate PR.

@mbrt

mbrt commented Aug 11, 2022

Copy link
Copy Markdown
Owner

That's totally fine. Thanks a lot!

@mbrt
mbrt merged commit a05e2a0 into mbrt:master Aug 11, 2022
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