Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Initial tests for slskmessages #42

Open
wants to merge 5 commits into
base: python3
Choose a base branch
from

Conversation

droserasprout
Copy link
Collaborator

First attempt to cover slskmessages module with unit tests and then clean up things a bit

@lene lene force-pushed the python3 branch 14 times, most recently from 657e1d0 to 9805bb6 Compare April 21, 2020 18:11
@toofar
Copy link

toofar commented Apr 24, 2020

Is the intent of these tests to show that the python3 migration hasn't broken anything or to allow for less guess work in refactoring? Because if it is the former they should be merged to upstream so they can be run against both versions and the latter I think it make sense to wait until after the python3 stuff is merged? To not make the PR even bigger and harder to review.
I'm not saying more tests are bad, just trying to temper enthusiasm for fixing everything at once in case the initial migration gets bogged down in feature creep. In particular I would like to add tests around the networking stuff to make refactoring it to be more efficient and handle more connections easier.

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