[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #5520 [Vidalia]: Test the new bootstrap procedure in Vidalia alpha



#5520: Test the new bootstrap procedure in Vidalia alpha
---------------------+------------------------------------------------------
 Reporter:  chiiph   |          Owner:  sebb          
     Type:  task     |         Status:  needs_review  
 Priority:  normal   |      Milestone:  Vidalia: 0.3.x
Component:  Vidalia  |        Version:                
 Keywords:           |         Parent:                
   Points:           |   Actualpoints:                
---------------------+------------------------------------------------------

Comment(by chiiph):

 Replying to [comment:2 sebb]:

 It's seems you've been busy :)

 > I've reached a point where I can really use a code review, there are few
 points that I'm not sure of:
 > - I'm new to CMake, so it'd be nice if someone more experienced verifies
 if the test code is integrated correctly into the project

 I'm not a test infrastructure guru, but here's what I know: there are a
 lot of ways of handling tests (see
 [http://www.cmake.org/Wiki/CMake/Testing_With_CTest CTest], or [http://qt-
 project.org/doc/qt-4.8/qtestlib-manual.html QTestLib])

 Are these what we need for Vidalia? I don't know. So, in terms of beauty,
 this might not be the best way to handle tests, but it might be good
 enough for now, or perfect for an application like Vidalia.

 I guess you would have to ask yourself this: how hard would it be to add a
 test for the settings handling? If the answer is "well, quite simple",
 then we are good.

 > - there are few places in the main vidalia code I needed to alter in
 order to make test code working, I've left comments there
 > - bootstrap code in vidalia is simplified too, with use of signals /
 slots (we talked about it some time ago on irc), it looks like it works
 correctly, I'm curious to know your opinion on this implementation

 Have you written a sort of execution graph for the different stages of the
 bootstrap procedure and every possible scenario? I'd like to see that (or
 something similar) for your code (I'm not asking you to open photoshop or
 anything :), just some text or simply tell me "yes, I've carefully
 considered every possible scenario, trust me").
 I found out doing this a couple of somewhat signal/slot race conditions
 that I didn't like, and that's why I went with fully controlled async.

 > This is not the final version, its more like I need to know if this is
 going in good direction, so any hints are welcome.

 I'll take a closer look to the code tomorrow. Either way, great work! Keep
 it up!

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/5520#comment:3>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs