Skip to content
This repository has been archived by the owner on Dec 17, 2024. It is now read-only.

assert_tokens has expected and actual arguments swapped #16

Closed
mkpankov opened this issue Apr 23, 2018 · 2 comments
Closed

assert_tokens has expected and actual arguments swapped #16

mkpankov opened this issue Apr 23, 2018 · 2 comments

Comments

@mkpankov
Copy link

Having the following failed currently produces error message that seems incorrect.

    assert_tokens(&v1, &[
        Token::Bytes(&[b'a', b'b', b'c']),
    ]);

Error:

thread 'main' panicked at 'expected Token::Str("abc") but serialized as Bytes([97, 98, 99])', /home/mkpankov/.cargo/registry/src/github.com-1ecc6299db9ec823/serde_test-1.0.42/src/ser.rs:161:18

I see second argument as expected, and first as one being tested (actual), so I wanted this message:

thread 'main' panicked at 'expected Bytes([97, 98, 99]) but serialized as Token::Str("abc")', /home/mkpankov/.cargo/registry/src/github.com-1ecc6299db9ec823/serde_test-1.0.42/src/ser.rs:161:18

Swapping the arguments in call site doesn't work:

error[E0308]: mismatched types
  --> hlua-serde-test/src/main.rs:32:8
   |
32 |     ], &v1);
   |        ^^^ expected slice, found enum `hlua::AnyLuaValue`
   |
   = note: expected type `&[serde_test::Token]`
              found type `&hlua::AnyLuaValue`

error[E0277]: the trait bound `serde_test::Token: serde::ser::Serialize` is not satisfied
  --> hlua-serde-test/src/main.rs:30:5
   |
30 |     assert_tokens(&[
   |     ^^^^^^^^^^^^^ the trait `serde::ser::Serialize` is not implemented for `serde_test::Token`
   |
   = note: required because of the requirements on the impl of `serde::ser::Serialize` for `[serde_test::Token; 1]`
   = note: required by `serde_test::assert_tokens`

error[E0277]: the trait bound `serde_test::Token: serde::de::Deserialize<'_>` is not satisfied
  --> hlua-serde-test/src/main.rs:30:5
   |
30 |     assert_tokens(&[
   |     ^^^^^^^^^^^^^ the trait `serde::de::Deserialize<'_>` is not implemented for `serde_test::Token`
   |
   = note: required because of the requirements on the impl of `serde::de::Deserialize<'_>` for `[serde_test::Token; 1]`
   = note: required by `serde_test::assert_tokens`

error: aborting due to 3 previous errors

error: Could not compile `hlua-serde-test`.

So that means second argument is indeed meant to be the expected, and failed assert message is incorrect.

I tried looking at specific place that outputs the message (https://github.com/serde-rs/serde/blob/9bc05803feef2e2ee30c62cb047d7c3ea6d44fbe/serde_test/src/ser.rs#L74) but couldn't make sense of argument names in the macro, seems just swapping them in format string would make the code confusing (expected is called other?).

@dtolnay
Copy link
Contributor

dtolnay commented Apr 23, 2018

Thanks! I would accept a PR to improve the assertion message and naming of variables in the macro.

@dtolnay
Copy link
Contributor

dtolnay commented Jul 9, 2023

Fixed in serde_test 1.0.122 by serde-rs/serde#1918.

@dtolnay dtolnay closed this as completed Jul 9, 2023
@dtolnay dtolnay transferred this issue from serde-rs/serde Aug 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants