status-im / nim-stew

stew is collection of utilities, std library extensions and budding libraries that are frequently used at Status, but are too small to deserve their own git repository.
139 stars 18 forks source link

Splitting `all_tests` wrapper #124

Closed mjfh closed 2 years ago

mjfh commented 2 years ago

Running some unit tests in a separate file attempts to mitigate the global symbols overflow on Github ci when compiling with nim version <= 1.2.

arnetheduck commented 2 years ago

the error in the test indicates that there are too many globals - the better fix would be to reduce the overall count of globals by moving them into test - this also makes for better tests that are more independent - in particular, it seems that the new collections are producing many globals

mjfh commented 2 years ago

Could you please explain or give an example? As far as I understand, the latest addition has not many globals. So it was just the literal straw that broke the camel's neck.

arnetheduck commented 2 years ago

3 things should be done:

mjfh commented 2 years ago

ok, I see.

I guess unittest2 was avoided in order to reduce the imported packages? Otherwise i would prefer to use unittest2 and wrap in the main function wrapper.

arnetheduck commented 2 years ago

I'm guessing it hasn't been needed until now - @zah started using it in https://github.com/status-im/nim-stew/pull/122 (stew predates unittest2) - when migrating, it's best to do all unittests in one go so as to avoid issues - it's mostly doable with a search/replace of the import

mjfh commented 2 years ago

Well, then I prefer to do it now. I see that is the most clean solution.

It is widely used in nim-eth though there are pockets using testutils/unittest.

mjfh commented 2 years ago

.. or tomorrow morning :)

mjfh commented 2 years ago

superseded by #125