Review Board 1.0.5.1

Supybot Integration (Part 1)

Updated 9 months, 1 week ago

Andrew Schurman Reviewers
Basie-Dev
None Basie SVN
This is the initial integration of supybot with the ability to use django to automate testing of supy plugins. It offers no functionality of the bot itself, but merely allows one to automate the supy plugin test cases. The idea is to use a dummy application to facilitate testing of supy plugins. The dummy application will likely be used for callbacks back into django later. Specific functionality to be determined.

This requires the patch from http://review.basieproject.org/r/418/

There are a few places where I've set the PYPATH manually. Is there a better way to handle this?

Bugs:
- Generates dummy configuration dirs in the working dir (likely fixed with next patch)
- An existing project with ID=0 must exists (requirement from http://review.basieproject.org/r/418/ )
- When running the tests, django reports 7 tests, but there is only 5 (the 2 extra tests pass though)
- Deleted /eggs/ from the base basie dir and ran install.sh. Supybot lib was installed as expected. `bin/buildout' will do the same thing (without deleting eggs).
- Ran `bin/django test -- supybot_callbacks' to initiate the tests. All passed.
Posted 9 months, 3 weeks ago (October 7th, 2009, 5:37 p.m.)

   

  
/trunk/apps/supybot_callbacks/tests/__init__.py (Diff revision 1)
 
 
 
 
 
 
 
Why can't we just `import settings' in a test?
Using an external resource instead of creating the file seems more appealing, but we need to set the platform dependant paths! Supy provides hooks to change these after reading the registry.
Posted 9 months, 3 weeks ago (October 8th, 2009, 10:22 a.m.)

   

  
Now that I think about it, this means class names need to be unique. Not a bad constraint, but may be overlooked down the road.

I haven't found a way to get django to auto discover a test suite instead of just test cases. That would've been more ideal.
Posted 9 months, 3 weeks ago (October 8th, 2009, 11:26 a.m.)

   

  
Are you sure you want a supybot to reply to non-commands? On that note, do you want the bot to listen to commands at all, if it's there just to be logging? Could be a security risk if the attacker figures out a way to do something malicious to the server through a bot.
  1. Good catch--copy and paste from the test script. Not too much harm since we are only testing plugins, but we should add a test for this case.
Review request changed
Updated 9 months, 1 week ago (October 20th, 2009, 1:10 p.m.)
Added changes after updating svn to head.
Review request changed
Updated 9 months, 1 week ago (October 21st, 2009, 6:12 p.m.)
Move from /apps/supybot_callbacks to /apps/ircbot. Can't use supybot as the app name due to namespace clash with supybot namespace. Unified plugin dir resolution in ircbot.tests.__init__.
Posted 9 months, 1 week ago (October 21st, 2009, 8:44 p.m.)

   

  
Are django templates available at this point? If they are available you can create that test configuration using a template. Ex. apps/tickets/templates/tickets/index_content.txt and apps/tickets/index.py line 24
Can these import lines be at the top?
  1. A thought occurred, do we want the package to expose these imported modules? If we `import os` in this script, then we can go test.os.mkdirs or something.
I the future this filter will be used not only for tests right?
I like these docstrings :)
/trunk/buildout.cfg (Diff revision 3)
 
There is a problem with this dependence. I got an error while running buildout. I tried to decompress the tar.gz in basie_eggs and there was the same error too:

gzip: stdin: invalid compressed data--format violated
tar: Child returned status 1
tar: Error exit delayed from previous errors
Posted 9 months, 1 week ago (October 22nd, 2009, 10:22 a.m.)

   

  
/trunk/apps/ircbot/tests/__init__.py (Diff revision 3)
 
 
Nice. Was thinking of using templates, but didn't know how it would be accomplished.
/trunk/apps/ircbot/tests/__init__.py (Diff revision 3)
 
 
Some of them can, but others can't (like supybot.conf) as it depends on other items already loaded into memory. Rather than try to follow this convention, I think its best to leave as is.
  1. Ok.
/trunk/apps/ircbot/tests/__init__.py (Diff revision 3)
 
 

     

  
/trunk/apps/ircbot/tests/plugintest.py (Diff revision 3)
 
 
I wish I could take credit. They (including the methods) are from django.
/trunk/buildout.cfg (Diff revision 3)
 
 
Sound like I messed up the binary diff or you didn't apply the binary diff properly, but it shouldn't affect an svn commit. I can try another diff, but you can just get the file from http://sourceforge.net/projects/supybot/files/supybot/Supybot-0.83.4.1/Supybot-0.83.4.1.tar.gz/download

The archive I have on my machine works.
  1. I did that and I was able to run the tests (I applied the two ircbot patches):
    
    1) bin/django test -- -w apps/ircbot/
    Ran 2 tests in 0.004s
    OK
    
    2) bin/django test -- -w apps/ircbot/plugins/BasieLogger/
    Ran 6 tests in 9.812s
    FAILED (errors=6)
    
    http://pastie.org/665394
    
    What I did wrong? It is nice that those errors are not when I run 'bin/django test' but I still want to ran the BasieLogger tests, even if it is with a particular command.
  2. As for (1), that's quite weird, only 2 tests? There should be 8 or something. I tried your command and have 8 of them passing. I don't think theres any difference in the command `bin/django test -- ircbot` as it does the same thing for me.
    
    The problem with (2) is that all test initialization happens in ircbot.test.__init__. I could modify the plugin so that it does load up that module and write the test configuration, but there's no way to enforce it completely.
  3. Hmm... nose won't use the setup scripts for packages that are imported. The only solutions are to leave as is or to change our the test packages init to not be a nose setup script but a global package init script. I think the former would be a bit cleaner unless we are gonna drop nose in the future.
  4. Ok, lets check that later (after you commit). Let me read the new patch and probably you will get a shipt it here.
Review request changed
Updated 9 months, 1 week ago (October 22nd, 2009, 10:35 a.m.)
Update with changes as per comments.
Ship it!
Posted 9 months, 1 week ago (October 22nd, 2009, 1:16 p.m.)
Nice.
these should be in /trunk/apps/ircbot/templates/ircbot/test.conf (the idea is that the template name does not conflicts with templates in others app so we use the app name as prefix).

You can change this before the commit (you don't need to update the diff on review board)
Then this will be render_to_string('ircbot/test.conf',...)