serde-rs/test

assert_tokens has expected and actual arguments swapped

Closed this issue · 2 comments

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?).

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

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