search

cointop-sh / cointop

A fast and lightweight interactive terminal based UI application for tracking cryptocurrencies 🚀
https://cointop.sh
Apache License 2.0
4.05k stars 314 forks source link

Fix CMC Coin ID #282

Closed vuon9 closed 3 years ago

vuon9 commented 3 years ago

279

lyricnz commented 3 years ago

Any idea how we can test this?

vuon9 commented 3 years ago

Which case u want to test? I think can do by unit tests.

lyricnz commented 3 years ago

I wasn't trying to be too fussy, just trying to figure out how we could exercise it, to know the code is OK. I ran the branch with cmc, seemed to work OK, but the bug was around stale caches etc.

vuon9 commented 3 years ago

Seems functional test with caching behavior, not about this fix, but about any fix relates to cached data, so I believe it's not simple. Assume it needs to warm up cache for the previous version of CMC coin data (ID = Name), then call the func to get coin (which prioritized cached data first), then have the comparison for actual/expect data.

What if the app just clear cache to have a fresh state by release? We may control it in a config file, like cache version or something like that. More than that, benefit in not to add clearing cache logic so many times if the fix relates to the cached data.

Until we have it, I may need to add clearing cache here, as I did in coinlink PR.

lyricnz commented 3 years ago

Created #291 to add feature for auto cache-clear. Happy to merge/close this one, but let's get 291 done before the next release.