woocommerce-android
woocommerce-android copied to clipboard
Tests that rely on `Order.EMPTY` are time-sensitive
All unit tests that use Order.EMPTY are now based on time, as since #6534 the time of creation is generated on each get.
This means that when executing tests, we can't compare Order.EMPTY as it'll be different each time it's requested.
IMO we should make Order.EMPTY time agnostic (assign a default value for date, e.g. Instant.EPOCH) and introduce injectable java.time.Clock which will be a fixed one in tests.
Also maybe it's a naming issue. Because Order.EMPTY.isEmpty() now returns false.
Hi @wzieba,
I noticed this issue and find the idea of making Order.EMPTY time agnostic very interesting. Introducing a default date value like Instant.EPOCH and an injectable java.time.Clock for fixed values during tests sounds like a great approach to improve consistency and test reliability.
Would it be alright if I take this up? I'd be happy to work on implementing this and addressing the naming concern regarding Order.EMPTY.isEmpty().
hi @Akshaykomar890 👋 thanks for being interested in improving this. As I'm not working on this project actively anymore, let me cc @malinajirka to provide their insight here.
Hello again @Akshaykomar890 👋,
thanks again for your interest in contributing! Feel free to work on this issue.
I don't have much insights into the complexity of this issue, but I like the idea and the solution proposed by Wojtek sounds like a good one. Let me know in case you encounter any issues and I'll be happy to chat about them with you (and hopefully help you resolve them :P ).
Hello @malinajirka,
Thanks for your previous feedback and support. #13478 I've implemented the proposed changes and submitted my PR. Could you please take a look and review it when you have a moment? Let me know if there are any issues or further adjustments needed.
Thanks again