Github user arina-ielchiieva commented on the issue:
https://github.com/apache/drill/pull/904
+1, LGTM.
---
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/904
@weijietong, thanks for the pull request, +1
---
Github user weijietong commented on the issue:
https://github.com/apache/drill/pull/904
done
---
Github user weijietong commented on the issue:
https://github.com/apache/drill/pull/904
applied the review comments
---
Github user weijietong commented on the issue:
https://github.com/apache/drill/pull/904
@vvysotskyi any more advice ?
---
Github user weijietong commented on the issue:
https://github.com/apache/drill/pull/904
@vvysotskyi thanks for your help! have done, please review.
---
Github user weijietong commented on the issue:
https://github.com/apache/drill/pull/904
@vvysotskyi thanks for your hard work, I have rewritten the corresponding
method. But I have not rewritten the other Locate related cases. Since mockup
is a white box testing , I payed some
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/904
@weijietong thanks for the explanation of your problem. I was able to
reproduce it, but also I found working solution. This mock works correctly. The
problem appears when unit test is run with
Github user weijietong commented on the issue:
https://github.com/apache/drill/pull/904
@vvysotskyi the problem with mocking strategy is that when I run the test
case just with the single method `testToDateForTimeStamp` it will pass. but
when I run the whole test cases of the
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/904
@weijietong which problems did you have when tried to mock
`System.getProperty()` method?
As the example of nice mock of this method, you may use [this
Github user paul-rogers commented on the issue:
https://github.com/apache/drill/pull/904
@vvysotskyi, can you take a look at the revisions and see if it looks OK?
---
Github user weijietong commented on the issue:
https://github.com/apache/drill/pull/904
@vvysotskyi thanks for your patient review . As you pointed out , the
timezone would not take effect when test cases run in parallel. I also tried
the mock strategy. It will also fail in
Github user weijietong commented on the issue:
https://github.com/apache/drill/pull/904
@vvysotskyi has updated the mentioned parts. These
[4ea36c3](https://github.com/apache/drill/commit/4ea36c3f18841b229aca5048b7d162ea16bfd5a1),
Github user weijietong commented on the issue:
https://github.com/apache/drill/pull/904
@vvysotskyi please review the update ones
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this
14 matches
Mail list logo
