Test the new bootstrap procedure in Vidalia alpha
Vidalia 0.3.2-rc has a new implementation for the bootstrap procedure. It'd be nice to have it intensively tested.
Basically, the idea would be to take every stage of the bootstrap procedure and stress it out mainly with "tor dies" in each of them. It also handles other kinds of errors but the timing makes it somehow difficult to test, so trying to make sure each of those erroneous situations are in control would be good.
Activity
changed milestone to %Vidalia: 0.3.x
here's how i can break the tor startup procedure (it worked each time i tested):
- setup a relay (tested with non-exit relay), disable automatic tor loading
- delete "cached-*" files from "data" directory
- start vidalia, click "start tor"
- wait until "Loading relay information" (~61%), then click "stop tor" - nothing happens
- click "stop tor" again, everything looks like it should now ("tor is not running" icon etc), but you can't exit vidalia - "exit" menu entry does not work tor version : tor-browser-2.3.12-alpha-1_en-US vidalia : latest alpha build OS: windows xp sp3 btw. i haven't finished implementing automated tests yet, i hope i can find some time in the following week
Trac:
Username: sebb-
https://github.com/sebthestampede/vidalia/commit/2f629bd0c558544285e413a1ff9e084078be1d13 branch: 5520_bootstrap_test
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
- 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 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.
Trac:
Username: sebb
Status: new to needs_review Replying to 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 CTest, or 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!
-
This testing code uses QtTest framework, so adding new tests is quite straightforward - it requires to add another class to CMakeList and one method call in testMain.cpp. I don't have execution graph for the bootstrap procedure. Do you remember where exactly we can expect a race condition, maybe you still have your notes left somewhere ?
Trac:
Username: sebb You use QTestLib, but the tests don't get executed as part of the building procedure. Which is fine, but you asked if it was integrated correctly and the answer is "in terms of developing tests it can be integrated even more, but if it tests things correctly I don't have a problem with executing another application after I rebuild Vidalia".
Regarding the execution graph, one that I can remember was the Circuit Established phase, which can be called twice if the signal is emitted just before a certain method was called where circuitEstablished() was invoked.
The other issue I didn't like is that there are a lot of possible signals emitted from TorControl at any given point, and from the MainWindow it's kind of hard to debug certain issues because you don't have a clear idea which methods where executed when. The graph isn't clear from the code, and that was one of my goals when rewriting that part.
That doesn't mean my solution doesn't have problems, the idea with this task was to change the "it looks like it works" to "I'm 99% certain it works, because I have tools to test every possible scenario I can think of". So if your solution falls in the second sentence, then it's good for me. If you don't know, you need to look at the execution graph and make sure your tests have all that covered.
-
Ok, it seems that you've already been there, done that and i'm moving too far away from the original ticket goal. I'll just revert the changes and leave the bootstrap procedure as it is, changing the implementation deserver at least a separate ticket, maybe someday someone will want to play with it more. I don't have problems with it if it works and is maintainable, and I guess there are more important issues i could take care of for the alpha version. I hope I can update the branch in few days, lately I'm learning the true meaning of the word "deadline", the hard way ;)
Trac:
Username: sebb
Status: needs_review to needs_revision -
https://github.com/sebthestampede/vidalia/commit/af23c292c56e9813c80f3823aa8322b8550c8202 branch is: 5520_bootstrap_test
Trac:
Username: sebb I get this when trying to test it:
make[2]: *** No rule to make target `/Users/chiiph/Code/vidalia/src/vidalia/i18n/vidalia_ar.qm', needed by `src/tests/qrc_vidalia_i18n.cxx'. Stop. make[1]: *** [src/tests/CMakeFiles/VidaliaTestSuite.dir/all] Error 2 make: *** [all] Error 2Trac:
Status: needs_review to needs_revision-
Trac:
Username: sebb -
https://github.com/sebthestampede/vidalia/commit/9f922d20ccfd78fc4b222a051c99caed383c5512 branch: 5520_bootstrap_test
now it has improved timeout functionality, so it wont stuck on a single test case again
"i'll check it again as soon as i can" :D
Trac:
Username: sebb
Status: needs_revision to needs_review