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

Change unit test of stratum to use unused ports #457

Closed
remagpie opened this issue Jul 23, 2018 · 6 comments
Closed

Change unit test of stratum to use unused ports #457

remagpie opened this issue Jul 23, 2018 · 6 comments
Labels

Comments

@remagpie
Copy link
Contributor

Some unit tests in stratum crate opens network socket.
e.g.

Stratum::start(&addr, Arc::new(DummyManager::build().of_initial(r#"["dummy autorize payload"]"#)), None)

While it works and serves its own purpose, it's not a good practice to depend on external environment in unit test.
For example, if socket is already open by other application, the test will fail even if there's nothing wrong with its logic.
We should remove such tests, and find other way to test functionalities in those files.

@hyunsikjeong
Copy link
Contributor

@Kais-DkM I think it is inevitable by the structure of Stratum, but we can ensure the unit test works by checking socket addresses and using a socket address which is not used now.

@remagpie
Copy link
Contributor Author

@jhs7jhs Then what would be the correct operation when the port is already occupied?

@hyunsikjeong
Copy link
Contributor

hyunsikjeong commented Nov 29, 2018

@Kais-DkM Just find another port to open. "using a socket address which is not used now"

@remagpie
Copy link
Contributor Author

@jhs7jhs It seems we have to choose a range of ports for testing. Do you have any value in mind?

@hyunsikjeong
Copy link
Contributor

@Kais-DkM How about the range 19900 ~ 19999? The current test cases are using 19970, 19975, 19990, 19995, 19991. I don't know reason why, but 100 ports should be enough to test, as I think.

@remagpie
Copy link
Contributor Author

I agree with that. It seems good enough if there's no problem.

@hyunsikjeong hyunsikjeong changed the title Don't open socket in unit test Change unit test of stratum to use unused ports Nov 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants