Skip to content

Splitting all_tests wrapper#124

Closed
mjfh wants to merge 1 commit intomasterfrom
mitigate-ci-symbols-overflow
Closed

Splitting all_tests wrapper#124
mjfh wants to merge 1 commit intomasterfrom
mitigate-ci-symbols-overflow

Conversation

@mjfh
Copy link
Contributor

@mjfh mjfh commented Jul 7, 2022

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.

why:
  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
Copy link
Member

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
Copy link
Contributor Author

mjfh commented Jul 7, 2022

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
Copy link
Member

arnetheduck commented Jul 7, 2022

3 things should be done:

@mjfh
Copy link
Contributor Author

mjfh commented Jul 7, 2022

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
Copy link
Member

I'm guessing it hasn't been needed until now - @zah started using it in #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
Copy link
Contributor Author

mjfh commented Jul 7, 2022

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
Copy link
Contributor Author

mjfh commented Jul 7, 2022

.. or tomorrow morning :)

@mjfh
Copy link
Contributor Author

mjfh commented Jul 8, 2022

superseded by #125

@mjfh mjfh closed this Jul 8, 2022
@mjfh mjfh deleted the mitigate-ci-symbols-overflow branch July 8, 2022 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants