search

typeddjango / pytest-mypy-plugins

pytest plugin for testing mypy types, stubs, and plugins
https://pypi.org/project/pytest-mypy-plugins/
MIT License
102 stars 24 forks source link

Refactor and fix assert_expected_matched_actual #65

Closed zero323 closed 3 years ago

zero323 commented 3 years ago

This PR:

The following test file:

- case: all_mismatched
  main: |
    reveal_type(42)  # N: Revealed type is "Literal['foo']?"
    reveal_type("foo")  # N: Revealed type is "Literal[42]?"

- case: missing_message_then_match
  main: |
    reveal_type(42)
    reveal_type("foo")  # N: Revealed type is "Literal['foo']?"

- case: match_then_missing_message
  main: |
    reveal_type(42)  # N: Revealed type is "Literal[42]?"
    reveal_type("foo")

- case: missing_message
  main: |
    42 + "foo"

- case: mismatched_message_inline
  main: |
    1 + 1  # E: Unsupported operand types for + ("int" and "int")

- case: mismatched_messaged_in_out
  main: |
    1 + "foo"
  out: |
    main:1: error: Unsupported operand types for + ("int" and "int")

- case: match_then_mismatched_message
  main: |
    reveal_type(42)  # N: Revealed type is "Literal[42]?"
    reveal_type("foo")  # N: Revealed type is "builtins.int"

- case: mismatched_message_then_match
  main: |
    reveal_type("foo")  # N: Revealed type is "builtins.int"
    reveal_type(42)  # N: Revealed type is "Literal[42]?"

- case: match_between_mismatched_messages
  main: |
    reveal_type(42.0)  # N: Revealed type is "builtins.float"
    reveal_type("foo")  # N: Revealed type is "builtins.int"
    reveal_type(42)  # N: Revealed type is "Literal[42]?"

has been used to check for expected failures and gives output as shown below

test-expect-fail.yaml FFFFFFFFF                                                     [100%]

======================================== FAILURES =========================================
_____________________________________ all_mismatched ______________________________________
/home/zero323/Workspace/test-reg/failing/test-expect-fail.yaml:3: 
E   pytest_mypy_plugins.utils.TypecheckAssertionError: Invalid output: 
E   Actual:
E     main:1: note: Revealed type is "Literal[42]?" (diff)
E     main:2: note: Revealed type is "Literal['foo']?" (diff)
E   Expected:
E     main:1: note: Revealed type is "Literal['foo']?" (diff)
E     main:2: note: Revealed type is "Literal[42]?" (diff)
E   Alignment of first line difference:
E     E: ...ed type is "Literal['foo']?"
E     A: ...ed type is "Literal[42]?"
E                               ^
_______________________________ missing_message_then_match ________________________________
/home/zero323/Workspace/test-reg/failing/test-expect-fail.yaml:9: 
E   pytest_mypy_plugins.utils.TypecheckAssertionError: Invalid output: 
E   Actual:
E     main:1: note: Revealed type is "Literal[42]?" (diff)
E     main:2: note: Revealed type is "Literal['foo']?" (diff)
E   Expected:
E     main:2: note: Revealed type is "Literal['foo']?" (diff)
E   Alignment of first line difference:
E     E: main:2: note: Revealed type is "Literal['foo']?"
E     A: main:1: note: Revealed type is "Literal[42]?"
E             ^
_______________________________ match_then_missing_message ________________________________
/home/zero323/Workspace/test-reg/failing/test-expect-fail.yaml:12: 
E   pytest_mypy_plugins.utils.TypecheckAssertionError: Invalid output: 
E   Actual:
E     main:2: note: Revealed type is "Literal['foo']?" (diff)
E   Expected:
E     (empty)
_____________________________________ missing_message _____________________________________
/home/zero323/Workspace/test-reg/failing/test-expect-fail.yaml:17: 
E   pytest_mypy_plugins.utils.TypecheckAssertionError: Output is not expected: 
E   Actual:
E     main:1: error: Unsupported operand types for + ("int" and "str") (diff)
E   Expected:
E     (empty)
________________________________ mismatched_message_inline ________________________________
/home/zero323/Workspace/test-reg/failing/test-expect-fail.yaml:22: 
E   pytest_mypy_plugins.utils.TypecheckAssertionError: Invalid output: 
E   Actual:
E     (empty)
E   Expected:
E     main:1: error: Unsupported operand types for + ("int" and "int") (diff)
_______________________________ mismatched_messaged_in_out ________________________________
/home/zero323/Workspace/test-reg/failing/test-expect-fail.yaml:26: 
E   pytest_mypy_plugins.utils.TypecheckAssertionError: Invalid output: 
E   Actual:
E     main:1: error: Unsupported operand types for + ("int" and "str") (diff)
E   Expected:
E     main:1: error: Unsupported operand types for + ("int" and "int") (diff)
E   Alignment of first line difference:
E     E: ...rand types for + ("int" and "int")
E     A: ...rand types for + ("int" and "str")
E                                        ^
______________________________ match_then_mismatched_message ______________________________
/home/zero323/Workspace/test-reg/failing/test-expect-fail.yaml:33: 
E   pytest_mypy_plugins.utils.TypecheckAssertionError: Invalid output: 
E   Actual:
E     ...
E     main:2: note: Revealed type is "Literal['foo']?" (diff)
E   Expected:
E     ...
E     main:2: note: Revealed type is "builtins.int" (diff)
E   Alignment of first line difference:
E     E: ...te: Revealed type is "builtins.int"
E     A: ...te: Revealed type is "Literal['foo']?"
E                                 ^
______________________________ mismatched_message_then_match ______________________________
/home/zero323/Workspace/test-reg/failing/test-expect-fail.yaml:37: 
E   pytest_mypy_plugins.utils.TypecheckAssertionError: Invalid output: 
E   Actual:
E     main:1: note: Revealed type is "Literal['foo']?" (diff)
E     ...
E   Expected:
E     main:1: note: Revealed type is "builtins.int" (diff)
E     ...
E   Alignment of first line difference:
E     E: ...te: Revealed type is "builtins.int"
E     A: ...te: Revealed type is "Literal['foo']?"
E                                 ^
____________________________ match_between_mismatched_messages ____________________________
/home/zero323/Workspace/test-reg/failing/test-expect-fail.yaml:43: 
E   pytest_mypy_plugins.utils.TypecheckAssertionError: Invalid output: 
E   Actual:
E     ...
E     main:2: note: Revealed type is "Literal['foo']?" (diff)
E     ...
E   Expected:
E     ...
E     main:2: note: Revealed type is "builtins.int" (diff)
E     ...
E   Alignment of first line difference:
E     E: ...te: Revealed type is "builtins.int"
E     A: ...te: Revealed type is "Literal['foo']?"
E                                 ^
================================= short test summary info =================================
FAILED test-expect-fail.yaml::all_mismatched - 
FAILED test-expect-fail.yaml::missing_message_then_match - 
FAILED test-expect-fail.yaml::match_then_missing_message - 
FAILED test-expect-fail.yaml::missing_message - 
FAILED test-expect-fail.yaml::mismatched_message_inline - 
FAILED test-expect-fail.yaml::mismatched_messaged_in_out - 
FAILED test-expect-fail.yaml::match_then_mismatched_message - 
FAILED test-expect-fail.yaml::mismatched_message_then_match - 
FAILED test-expect-fail.yaml::match_between_mismatched_messages - 
==================================== 9 failed in 2.77s ====================================
zero323 commented 3 years ago

Can you please add a testcase that was failing for you to our suite?

Sure, but I'll need some guidance here. All the cases covered here (refactoring aside) are suppose to fail, so we cannot simply add these to pytest_mypy_plugins/tests/. Normally , it would be something that would be marked with xfail I guess. But how would you like to proceed here?

sobolevn commented 3 years ago

I guess we need something like xfail. Probably we can start with xfail: true in the test's metadata.

zero323 commented 3 years ago

@sobolevn I am going to convert it to draft.

While bringing back leading / trailing indicator I detected some discrepancies that might require further work. Specifically I was looking at failing_case_3 (cases where there is partial match and then we expect nothing) and started to wonder if zipping lines is a good approach in the first place.

It seems like any preceding mismatch will actually brake all the following matches. Let's say I have following case:

- case: break_following
  main: |
    reveal_type(1 + 1) 
    reveal_type(1 + 1) # N: Revealed type is "builtins.int"
    reveal_type(1 + 1) # N: Revealed type is "builtins.int"
    reveal_type(1 + 1) # N: Revealed type is "builtins.int"
    reveal_type(1 + 1) # N: Revealed type is "builtins.int"

While testing, pytest-mypy-plugins 1.9.1 will give us following result:

E   pytest_mypy_plugins.utils.TypecheckAssertionError: Invalid output: 
E   Expected:
E     <45 (diff)
E     <45 (diff)
E     <45 (diff)
E     <45 (diff)
E   Actual:
E     main:1: note: Revealed type is "builtins.int" (diff)
E     main:2: note: Revealed type is "builtins.int" (diff)
E     main:3: note: Revealed type is "builtins.int" (diff)
E     main:4: note: Revealed type is "builtins.int" (diff)
E     main:5: note: Revealed type is "builtins.int" (diff)
E   
E   Alignment of first line difference:
E     E: main:2: note: Revealed type is "builtins.int"
E     A: main:1: note: Revealed type is "builtins.int"
E             ^

which is rather counter-intuitive, given that all expectations, but the first one, are satisfied. WDYT?

zero323 commented 3 years ago

After this patch it would be 

E   pytest_mypy_plugins.utils.TypecheckAssertionError: Output is not expected: 
E   Actual:
E     main:1: note: Revealed type is "builtins.int" (diff)
E     main:2: note: Revealed type is "builtins.int" (diff)
E     main:3: note: Revealed type is "builtins.int" (diff)
E     main:4: note: Revealed type is "builtins.int" (diff)
E     main:5: note: Revealed type is "builtins.int" (diff)
E   Expected:
E     main:2: note: Revealed type is "builtins.int" (diff)
E     main:3: note: Revealed type is "builtins.int" (diff)
E     main:4: note: Revealed type is "builtins.int" (diff)
E     main:5: note: Revealed type is "builtins.int" (diff)
E   Alignment of first line difference:
E     E: main:2: note: Revealed type is "builtins.int"
E     A: main:1: note: Revealed type is "builtins.int"
E

but it feels like we would actually want something around these lines:

E   pytest_mypy_plugins.utils.TypecheckAssertionError: Output is not expected: 
E   Actual:
E     main:1: note: Revealed type is "builtins.int" (diff)
E        ....
E   Expected:
E     (empty)
E       ....
E   Alignment of first line difference:
E     E: main:2: note: Revealed type is "builtins.int"
E     A: main:1: note: Revealed type is "builtins.int"
E

This also escalates, when we have matching blocks interleaved with failures (lets say we copy main block from the example break_following twice).

sobolevn commented 3 years ago

This looks correct to me: https://github.com/typeddjango/pytest-mypy-plugins/pull/65#issuecomment-938062045

E   pytest_mypy_plugins.utils.TypecheckAssertionError: Output is not expected: 
E   Actual:
E     main:1: note: Revealed type is "builtins.int" (diff)
E     main:2: note: Revealed type is "builtins.int" (diff)
E     main:3: note: Revealed type is "builtins.int" (diff)
E     main:4: note: Revealed type is "builtins.int" (diff)
E     main:5: note: Revealed type is "builtins.int" (diff)
E   Expected:
E     main:2: note: Revealed type is "builtins.int" (diff)
E     main:3: note: Revealed type is "builtins.int" (diff)
E     main:4: note: Revealed type is "builtins.int" (diff)
E     main:5: note: Revealed type is "builtins.int" (diff)
E   Alignment of first line difference:
E     E: main:2: note: Revealed type is "builtins.int"
E     A: main:1: note: Revealed type is "builtins.int"
E

This also escalates, when we have matching blocks interleaved with failures (lets say we copy main block from the example break_following twice).

I need more examples 🙂 I don't understand this one.

zero323 commented 3 years ago

need more examples 🙂 I don't understand this one.

Sorry, let me try to explain it better.

Let's assume that I added simple patch to current master 

diff --git a/pytest_mypy_plugins/utils.py b/pytest_mypy_plugins/utils.py
index dd8db72..b5f85a0 100644
--- a/pytest_mypy_plugins/utils.py
+++ b/pytest_mypy_plugins/utils.py
@@ -251,7 +251,7 @@ def assert_expected_matched_actual(expected: List[OutputMatcher], actual: List[s
             if i >= len(actual) or not expected[i].matches(actual[i]):
                 if first_diff < 0:
                     first_diff = i
-                error_message += "  {:<45} (diff)".format(expected[i])
+                error_message += "  {:<45} (diff)".format(str(expected[i]))
             else:
                 e = expected[i]
                 error_message += "  " + str(e)[:width]

and I have a test case like this 

- case: break_following_2
  main: |
    reveal_type(1 + 1) 
    reveal_type(1.0 + 2.0) # N: Revealed type is "builtins.float"
    reveal_type("foo" + "bar") # N: Revealed type is "builtins.str"

When I run tests I see:

E   pytest_mypy_plugins.utils.TypecheckAssertionError: Invalid output: 
E   Expected:
E     main:2: note: Revealed type is "builtins.float" (diff)
E     main:3: note: Revealed type is "builtins.str" (diff)
E   Actual:
E     main:1: note: Revealed type is "builtins.int" (diff)
E     main:2: note: Revealed type is "builtins.float" (diff)
E     main:3: note: Revealed type is "builtins.str" (diff)
E   
E   Alignment of first line difference:
E     E: main:2: note: Revealed type is "builtins.float"
E     A: main:1: note: Revealed type is "builtins.int"
E

If you analyze the test case, you'll see that actual state is like this:

line actual expected match
1 Revealed type is "builtins.int" ✘
2 Revealed type is "builtins.float" Revealed type is "builtins.float" ✓
3 Revealed type is "builtins.str" Revealed type is "builtins.str" ✓

however alignment message

E   Alignment of first line difference:
E     E: main:2: note: Revealed type is "builtins.float"
E     A: main:1: note: Revealed type is "builtins.int"

clearly shows that we start with comparing line 2 of expected and line 1 of actual.

This escalates to all the following lines and probably gets worse with multi-line messages (I wanted to investigate that, hence #66).

I am aware that this is consistent with behavior of the internal mypy test suite, which returns

Expected:
  main:2: note: Revealed type is "builtins.float" (diff)
  main:3: note: Revealed type is "builtins.str" (diff)
Actual:
  main:1: note: Revealed type is "builtins.int" (diff)
  main:2: note: Revealed type is "builtins.float" (diff)
  main:3: note: Revealed type is "builtins.str" (diff)

Alignment of first line difference:
  E: main:2: note: Revealed type is "builtins.float"
  A: main:1: note: Revealed type is "builtins.int"
          ^

for equivalent input, but it seems a bit counter-intuitive.

sobolevn commented 3 years ago

@zero323 thanks a lot for this great explanation! 👍 I think that this is fine for now. You can open a new issue for it, if you want to.

Let's fix the initial bug first.

zero323 commented 3 years ago

Let's fix the initial bug first.

Fair enough :)

I've updated tests and PR description with up-to-date output.

sobolevn commented 3 years ago

@zero323 just a quick thought: maybe we can write a couple of regular python unit-tests to make sure this works? Won't this be easier in this case? What do you think?

zero323 commented 3 years ago

@zero323 just a quick thought: maybe we can write a couple of regular python unit-tests to make sure this works? Won't this be easier in this case? What do you think?

Agreed. I was just thinking how to add tests for #66 and realized that run on YAML samples is not going to cut it. And here, we're interested as much about failure itself, as the actual output.

Two questions:

sobolevn commented 3 years ago

Just plain functions and depend on pytest test discovery

Plain functions are the best! ⭐

Do you think it makes sense to decouple pure logic and test actual output

I think that we should test the output, since it is the root issue.

zero323 commented 3 years ago

Thanks @sobolevn!