[GitHub] [avro] shaeqahmed commented on pull request #1839: AVRO-3621: [Rust] do not log each error while validating against union variants

[GitBox <gi...@apache.org>]

shaeqahmed commented on PR #1839:
URL: https://github.com/apache/avro/pull/1839#issuecomment-1228412617

   > Awesome!
   > 
   > I've made a small improvement to the panic message for `assert_not_logged()` because with `assert_ne!()` it was a bit confusing: `left != right. Left: X, Right: X`
   > 
   > Waiting for the CI to pass and I will merge it!
   
   Clarifying the message sounds good to me, but hm from what I understood the thread local LOG_MESSAGES is shared between multiple test cases sequentially, and only cleared at the end so merely checking for the presence of a `_last_log` without comparing it to the unexpected value wouldn't be correct right? I've updated with what I think should be the correct behavior:
   
   ```Rust
       match LOG_MESSAGES.borrow().last() {
           Some(last_log) if last_log == unexpected_message => panic!(
               "The following log message should not have been logged: '{}'",
               unexpected_message
           ),
           _ => (),
       }
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org