Skip to content
New issue

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

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

Already on GitHub? # to your account

[WFTC-58] getEstimatedTimeout to round down #67

Merged

Conversation

ochaloup
Copy link
Contributor

https://issues.jboss.org/browse/WFTC-58

  • adding a test

@dmlloyd: is the test appropriate here for the client. There is no tests so far. Maybe this is too simple and some more "integration" kind of test would be better? Or no test at all?

@ochaloup ochaloup force-pushed the WFTC-58-getestimated-time-round-down branch from 04a1e50 to 427f673 Compare January 22, 2019 12:45
@dmlloyd
Copy link
Member

dmlloyd commented Jan 22, 2019

I'd say the test is not good, as sleep is fairly unpredictable and also not desirable in tests in general; I think that static analysis is good enough for this change, but if we ever did want to do a white-box test, byteman would be the way to go. In general though I think white box unit tests cause us more difficulty than help.

@ochaloup ochaloup force-pushed the WFTC-58-getestimated-time-round-down branch from 427f673 to 7e21f98 Compare January 22, 2019 18:53
@ochaloup
Copy link
Contributor Author

@dmlloyd yeap, I pretty agree. It was not a good addition to the project. I left only the fix if that is ok from your point of view.

@dmlloyd dmlloyd merged commit ce1890e into wildfly:master Jan 22, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants