search

mergesort / Boutique

✨ A magical persistence library (and so much more) for state-driven iOS and Mac apps ✨
https://build.ms/boutique/docs
MIT License
899 stars 43 forks source link

Remove unnecessary Equatable constraint. #53

Closed connor-ricks closed 10 months ago

connor-ricks commented 1 year ago

Removes the unnecessary Equatable constraint on stored items as we can make use of the cacheIdentifier instead, which is already supposed to uniquely identify the object.

connor-ricks commented 1 year ago

Hmm... This test is passing for me locally.

Actually looks like this test is a little flakey on main and my branch here. 🤔

connor-ricks commented 1 year ago

I figured out what's going on.

It looks like your reset function dispatches work using an async Task. this results in a race condition as you are calling reset during your unit tests setup.

Screenshot 2023-05-19 at 12 35 58 AM
connor-ricks commented 1 year ago

Perhaps a change to .reset() is needed? 🤔

Alternatively a change could be made to the tests. You could put the @StoredValue inside each test.

mergesort commented 10 months ago

Thank you so much for the help @connor-ricks! As we discussed elsewhere I've gone ahead and made some changes today to address both the Equatable conformance and the @MainActor test flakiness. I think the change to remove Equatable conformance is a good one, and going forward @StoredValue operate on @MainActor which means we shouldn't run into this test flakiness.

Thanks again, sorry it took me a bit to get to, but I'm really grateful for the contribution!