Enable clippy during tests #153

Activity

1. aaronmondal

   self-assigned this

   on Jul 9, 2023
2. allada commented on Jul 12, 2023

   

   allada

   on Jul 12, 2023

   Contributor

   More actions

   Try and hold off on the scheduler stuff until @chrisstaite-menlo has landed his changes.

   👍React with 👍1Reacted by Aaron Siddhartha Mondal
3. allada commented on Jul 12, 2023

   

   allada

   on Jul 12, 2023

   Contributor

   More actions

   We "hold a RefCell reference across an await point" in cas/store/tests/filesystem_store_test.rs L731. Seems like something is wrong there.

   I ended up disabling this warning. I tried to turn it into a mutex but it didn't help, it just gave another clippy error. In this case the reason it's complaining is because we "might" deadlock because we are calling an async function while we hold a lock (for mutex). The RefCell is complaining because technically multiple people can mutate the object in random places in the code without a clear code path of where all has mutable references. Granted this would all happen on the same thread (because Rc is not Send). All it's trying to do is make sure that the promise is still pending and the way it does this is by wrapping the future in another future temporary and calling pretty much calling await on it... This allows the outer future to get a context and then it calls .poll() on the inner future, and returns the Polling results. In this case it's a test and I don't see a direct way to fix this without turning off the warning.
4. allada commented on Jul 12, 2023

   

   allada

   on Jul 12, 2023

   Contributor

   More actions

   The GetFinishedResult in cas/worker/tests/utils/mock_running_actions_manager.rs pointed out that the largest GetFinishedResult variant contains at least 496 bytes. It's talking about ActionResult here. I'd expect this to be fairly large but should it really be this large at all times?

   Hmmm, I can do some simple math on this:

   • 9 * sizeof(SystemTime)
   • 4 * sizeof(Vec<_>)
   • 2 * sizeof(DigestInfo)
   • 1 * i32
   • 1 * sizeof(HashMap<_, _>)
   • 1 * sizeof(String)
     https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=29c3e096a14d764a417e3197f19c6801

   Sums to 460, so yeah, my math is probably off a little because of DigestInfo's LazyTransform.
5. allada commented on Jul 12, 2023

   

   allada

   on Jul 12, 2023

   Contributor

   More actions

   FYI #174 almost certainly is going to trigger many places where we call .clone() but it's trivial copyable now, so we'll need to re-run clippy on many directories that were already fixed.

   👍React with 👍1Reacted by Aaron Siddhartha Mondal
6. aaronmondal commented on Jul 15, 2023

   

   aaronmondal

   on Jul 15, 2023

   ContributorAuthor

   More actions

   Closed by #182 Onwards to pedantic 🤣 🎉 🥳 🎈 🍰

   🎉React with 🎉1Reacted by Nathan (Blaise) Bruer
7. aaronmondal

   closed this as completedon Jul 15, 2023
8. rejuvenile

   added 4 commits that reference this issue on Aug 19, 2026

   small_blob_dispatcher: drainer + register_worker + TLA+ spec

   ...

   9feb479

   task TraceMachina#168: SmallBlobDispatcher integration plumbing (part…

   ...

   4e0521a

   small_blob_dispatcher_test: TDD failing tests for unpin_on_disconnect…

   ...

   3c6cfa2

   small_blob_dispatcher: delete pin_ttl + add unpin_on_disconnect (Trac…

   ...

   fb7a654