Write a test for NodeDetailsStatusUpdater
concentrate on bugfix #20994 (moved) and try to keep the test concise (it might be necessary to use reflection)
Activity
Do I need to create mock objects for ReverseDomainNameResolver, LookupService, Descriptor and other classes?
Do I need to use reflection to test private methods and do I even need to test private methods?
These are questions that I had sent to the metrics mailing list. It was inappropriate to send them there so I'm asking them here instead.
Trac:
Username: kiki201
Replying to kiki201:
Do I need to create mock objects for ReverseDomainNameResolver, LookupService, Descriptor and other classes?
Only, if you need these to test the fix supplied in #20994 (moved).
Do I need to use reflection to test private methods and do I even need to test private methods?
Same here, if it provides a shortcut and turns out to be easier you can use reflection, no must.
As the description of this ticket is quite terse, I try to give some more details:
Ticket #20994 (moved)#comment:5 explains the bug, its fix, and supplies the fixed code, but no test. So, the aim is to device a test that fails on code before this commit (and because of the reason described in #20994 (moved)#comment:5). There are no requirements of having to use mock objects or reflection, but they are fine to use for making the test code simpler. This might turn out tricky, just feel free to experiment.
-
Replying to iwakeh:
Replying to kiki201:
Do I need to create mock objects for ReverseDomainNameResolver, LookupService, Descriptor and other classes?
Only, if you need these to test the fix supplied in #20994 (moved).
Do I need to use reflection to test private methods and do I even need to test private methods?
Same here, if it provides a shortcut and turns out to be easier you can use reflection, no must.
As the description of this ticket is quite terse, I try to give some more details:
Ticket #20994 (moved)#comment:5 explains the bug, its fix, and supplies the fixed code, but no test. So, the aim is to device a test that fails on code before this commit (and because of the reason described in #20994 (moved)#comment:5). There are no requirements of having to use mock objects or reflection, but they are fine to use for making the test code simpler. This might turn out tricky, just feel free to experiment.
I didn't understand completely what the test is supposed to do. I am supposed to write a test that fails on the code before the given commit. Does the test checks if the first_seen timestamp on bridges is not "1970-01-01 00:00:00" ? Or does the test have more to do with the lines that were commented out in #20994 (moved)? Also, since this is on an old version of the code, I'm going to have to checkout in this branch. Is this correct?
Thank you
Trac:
Username: kiki201 -
Right, you can checkout the commit before the one given above or make the changes by hand; whatever you prefer. (there is only the master branch in the main repo.)
Regarding the bug in #20994 (moved): the error was to save nodes that never showed up in a consensus (cf. comment 3, there).
Just ask again, if anything is unclear.
-
So, the test should check if a consensus doesn't contain nodes that never showed up, so that the code before the commit fails. Is this right?
However, I'm not sure about how to go about testing this. The project contains a mock consensus object that implements RelayNetworkStatusConsensus. But, RelayNetworkStatusConsensus is only used in processDescriptor. So, I don't know how to take advantage of this class. I'm not sure how to simulate the whole process in order to test it.
Trac:
Username: kiki201
