[tor-dev] Migration of Tor Weather to Stem

[Sreenatha Bhatlapenumarthi]

Hi Damian,

Thanks for pointing out the controller socket error.
I've removed my old commits and added new ones at
https://github.com/lucyd/weather/commits/master.
Can you please take a look at them?

I haven't got a weather instance up but I followed the instructions in the
README to test it and 2 of the 9 tests failed.

======================================================================
FAIL: Test a subscribe attempt to all subscription types, relying
----------------------------------------------------------------------
Traceback (most recent call last):
  File
"/home/lucyd/Desktop/weather/weather/../weather/weatherapp/tests.py", line
311, in test_subscribe_all
    self.assertEqual(node_down_sub.grace_pd, 1)
AssertionError: 0 != 1

======================================================================
FAIL: Test a node down subscription (all other subscriptions off)
----------------------------------------------------------------------
Traceback (most recent call last):
  File
"/home/lucyd/Desktop/weather/weather/../weather/weatherapp/tests.py", line
89, in test_subscribe_node_down
    self.assertEqual(node_down_sub.grace_pd, 1)
AssertionError: 0 != 1

----------------------------------------------------------------------

It seems that an incorrect assert is causing the tests to fail.

self.assertEqual(node_down_sub.grace_pd, 1)

I looked it up and it appears that *grace_pd* is the default number of
hours which the subscriber's router must be offline before a notification
is sent and it's default value is zero and since it isn't set to any value
in the test, it assumed it's default value and the tests failed. I modified
the asserts as below and all the tests passed.

self.assertEqual(node_down_sub.grace_pd, 0)

Am I missing something? Or, are those tests incorrect?
Also, should I have a weather instance up and running and check manually
for email notifications to subscriptions or will the above tests suffice?

Cheers,
Sreenatha


On Sun, Jun 16, 2013 at 2:06 AM, Damian Johnson <atagar at torproject.org>wrote:

> > Hi,
> >
> >   My name is Sreenatha. I've made an attempt to migrate Tor Weather
> >   from TorCtl to Stem as stated in my gsoc proposal(Module 2).
> >
> >   Can you please take a look at the top 3 commits that I've made at my
> > github account?
> >   Please suggest any additional changes that you think are necessary.
> >
> > Cheers,
> > Sreenatha
> >
> > P.S : I am lucyd at OFTC on IRC.
>
> Hi Sreenatha, glad that you've started in on this! We'd much prefer
> for development discussions to be on tor-dev@ so looping that in.
>
> This looks like a great start. I suspect there is a little confusion
> though around the socket object that the Controller uses. The
> Controller doesn't take a socket.socket instance, but rather a
> stem.socket.ControlSocket...
>
> https://stem.torproject.org/api/socket.html
>
> This comes up a couple places, most notably...
>
>
> https://github.com/lucyd/weather/commit/ff22e74c247bd61163a974329cd2feb2a121db94#L0R26
>
> I doubt that function presently works. Rather, it should be...
>
> def listen():
>   controller = Controller.from_port(port = config.control_port)
>   controller.authenticate(config.authenticator)
>   controller.add_event_listener(newconsensus_listener,
> EventType.NEWCONSENSUS)
>
> I'm not sure what 'config.authenticator' is so that part might be
> wrong. Also note that this creates a control socket then forgets about
> it (Weather never cleans it up). This isn't a bug you've introduced
> but rather what Weather already did. Not the end of the world
> (interpreter shutdown will clean it up, though possibly with a
> stacktrace about lingering threads).
>
> Have you gotten a Weather instance up and running to test your changes?
>
> Cheers! -Damian
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.torproject.org/pipermail/tor-dev/attachments/20130705/de98c2ae/attachment.html>