Opened 7 years ago

Closed 6 years ago

#9874 closed enhancement (fixed)

Research/design a way to automate testing of BridgeDB's HTTPS and email distributors

Reported by: isis Owned by: isis
Priority: Medium Milestone:
Component: Circumvention/BridgeDB Version:
Severity: Keywords: bridgedb-unittests, automation, ci, bridgedb-gsoc-application, bridgedb-0.2.4
Cc: isis, sysrqb, trygve Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

We should find a way to write tests for the web interface and email responder. Currently, when I build a branch for testing, I run the unittests, then go over to the configured run/ directory, make/update some fake bridge descriptors, start bridgedb listening on localhost, then fiddle with various content settings like fonts and accepted languages, click on things, fiddle with URLs, and make sure nothing seems super broken. This should be automated.

Windmill (more docs) and twill look promising. Windmill probably wouldn't run very nicely on a headless server, however, as it drives a browser.

Find a way to write tests for the email responder. This could probably be a simple script to call sendmail, or it could be written in Twisted.

These tests don't need to be comprehensive immediately, just hack out something that works and is less crazy than doing it by hand another thousand times.

Child Tickets

Attachments (8)

bridgedb_https_test.py (3.3 KB) - added by trygve 6 years ago.
BridgeDB HTTPS Proof of Concept test
test_smtp.py (5.9 KB) - added by trygve 6 years ago.
BridgeDB SMTP auto-responder integration test suite (twisted.trial)
test_https.py (4.9 KB) - added by trygve 6 years ago.
BridgeDB HTTPS integration test suite (twisted.trial)
20140807_test_https.patch (2.8 KB) - added by trygve 6 years ago.
Fixed failing ipv6 test (replaces with vanilla bridge), adds scrameblesuit test
0001-Fixed-failing-test_get_obfs3_ipv6.patch (3.3 KB) - added by trygve 6 years ago.
0001-Raise-SkipTest-if-TESTING_BRIDGEDB-not-found-Fix-bri.patch (2.5 KB) - added by trygve 6 years ago.
Fixes to test_https.py
0002-Raise-SkipTest-if-TESTING_BRIDGEDB-not-found.patch (1.1 KB) - added by trygve 6 years ago.
Fixes to test_smtp.py
0003-Adds-missing-ipaddr-dependency.patch (733 bytes) - added by trygve 6 years ago.
Adds ipaddr to .test.requirements.txt

Download all attachments as: .zip

Change History (32)

comment:1 Changed 7 years ago by isis

There is also a PyXPCOM module, which may be useful, however that SE page says Windmill is the better way to go, and also mentions Mozmill (Mozilla's fork of Windmill). There appears to be some chance that we could get Windmill/Mozmill working on a remote server (which seems more appealing to me because then it could double for testing TBB builds).

Also, zope.testbrowser looks simple to use, works with Zope (what Twisted is built on top of), and only does browser simulation so it should run fine on a Travis CI machine or any other remote test server.

comment:2 Changed 7 years ago by isis

Keywords: automation ci added
Status: newneeds_information

comment:3 Changed 7 years ago by isis

Keywords: bridgedb-gsoc-application added
Parent ID: #9865

comment:4 Changed 6 years ago by isis

For one, I should state something I realised nearly a year ago so that no one else makes the same mistake: you can't drive TorBrowser with Selenium, because Selenium can't touch XUL elements. So while using Selenium would likely still work for testing all the major browsers for BridgeDB's HTTP(S) user interface, it wouldn't work for automated testing of BridgeDB's HTTP(S) UI in TorBrowser. We'll likely need both, in some capacity or another, because BridgeDB behaves differently if it believes that you're coming from a Tor exit relay (and we want to ensure that it is behaving differently for Tor and non-Tor users).

comment:5 Changed 6 years ago by isis

Additionally, because BridgeDB's SMTP server (the Twisted one) sits behind a postfix instance, it is sometimes necessary to test it from behind a mailserver. Here's an excerpt from a bridgedb.conf file explaining how to configure BridgeDB, and also how to start an Exim listener, in order to send emails which are forwarded through a mailserver to a test BridgeDB instance:

# IP and port to listen on for email connections. Debugging only.
EMAIL_BIND_IP = "127.0.0.1"
EMAIL_PORT = 6725

# To test the email server's responses with Exim, do:
#
# Set an /etc/hosts entry for the domain name of the client email address. For
# example, if your test emails (sent from ./scripts/telnet-an-email) are
# coming from 'alice@fubar', then set something like:
#
#     127.0.66.66 fubar
#
# in /etc/hosts. Next, start BridgeDB:
#
#     shellA $ bridgedb -r run-directory
#
# In another shell, start exim listening on the local IP you set in /etc/hosts:
#
#     shellB $ emim -bh 127.0.66.66:2525
#
# where 2525 is whatever random port (this should be neither EMAIL_PORT nor
# EMAIL_SMTP_PORT!). Then send the test email:
#
#     shellB $ ./scripts/telnet-an-email

And the telnet-an-email script is a crappy pexpect script which can be found here.

Last edited 6 years ago by isis (previous) (diff)

comment:6 Changed 6 years ago by isis

And... more things I've forgotten to mention on this ticket.

When testing BridgeDB, the usual way to run the tests (after installing and configuring BridgeDB and all its dependencies) is with Twisted's trial, by doing:

.../bridgedb/ $ trial ./lib/bridgedb/test/test_*.py


To obtain fake bridge descriptors for testing, use the leekspin module I wrote to generate them. (It requires pyOpenSSL and, optionally, pyNaCl, though the latter only if you want to generate ntor-onion-keys in the fake bridges' descriptors.)

comment:7 Changed 6 years ago by trygve

Proof of concept test for the BridgeDB web interface attached, written in Python. The test opens the BridgeDB web page on the local host, follows the 'bridges' link to '/options', modifies one of the 'advanced options', submits the form and inputs the correct value in the captcha. It then goes on to verify that at least one bridge line of the correct form is returned.

The ticket suggested the use of Windmill, twill or zope.testbrowser for testing the web interface.
For the proof of concept, I've used the mechanize and BeautifulSoup libraries because I've worked with them before. Note that both twill and zope.testbrowser are actually wrappers around mechanize (http://wwwsearch.sourceforge.net/mechanize/)

Note: the proof of concept does not use Twisted's trial. If the test is reviewed and found to be helpful, I can update it be consistent with the rest of the BridgeDB tests, and add a number of additional tests to exercise more of the BridgeDB code.

Last edited 6 years ago by trygve (previous) (diff)

Changed 6 years ago by trygve

Attachment: bridgedb_https_test.py added

BridgeDB HTTPS Proof of Concept test

comment:8 Changed 6 years ago by trygve

Running the test:

  • Ensure BridgeDB running in virtualenv on localhost with the default config file
  • Ensure valid bridge descriptors are available with at least one obfs2 bridge
  • Ensure the mechanize is installed in that virtualenv (pip install mechanize)
  • Run this script with that virtualenv i.e. "python bridgedb_https_test.py". Exceptions or assertion failures indicate that something has gone wrong ;) On success, bridge lines should be printed to the screen

comment:9 Changed 6 years ago by isis

Status: needs_informationneeds_revision

Looks good so far! It would need to run with BridgeDB's CI infrastructure, of course. They mention using Sauce Connect or xvfb. xvfb is probably the best way to go that requires the least amount of changes to your mechanize script.

comment:10 Changed 6 years ago by trygve

Status: needs_revisionneeds_information

Perhaps I've misunderstood, but Sauce Connect or xvfb both seem applicable to "above the browser" testing, whereas mechanize and derivatives (twill or zope.testbrowser) both function "below the browser", in the sense that they simulate a web browser but don't require one to be present. So mechanize works by opening URLs and interacting with the underlying HTML DOM received via HTTP/TCP, as opposed to simulating mouse events on the Browser in something like Selenium. The xvfb library stands for 'X Virtual Framebuffer', which doesn't sound like something which would be useful with mechanize.

I think Python + mechanize would be something that would be possible to integrate into Travis-CI, but I don't have any experience with this. There are a number of existing twisted.trial test scripts (run using "bridgedb test"), which I'm assuming already integrate into Travis-CI? If this is the case, it should be easy enough to add the mechanize tests to this framework. There is one significant difference compared to the other unit tests: mechanize actually requires the bridgedb application to be running, and is therefore more of an integration test than a unit test. But I've used unit test frameworks in the past to create suites of integration tests, so this shouldn't be an obstacle.

In light of the above, do you still feel your comment about Sauce Connect or xvfb is still applicable? If not, I'd like to work on incorporating this HTTPS test into the twisted.trial test suite. Also, I've got a Python script which can test the SMTP autoresponder working (it uses Python to send a message to the bridgedb SMTP server, and then listens on a port for the response, checking that an email is received that contains the text "Here are your bridges"). This also falls under the category of 'integration test'

Last edited 6 years ago by trygve (previous) (diff)

Changed 6 years ago by trygve

Attachment: test_smtp.py added

BridgeDB SMTP auto-responder integration test suite (twisted.trial)

comment:11 Changed 6 years ago by trygve

Status: needs_informationneeds_review

Attached 'test_smtp.py'. This is a test suite used to test the BridgeDB SMTP autoresponder. All tests are written to use twister.trial. The suite currently has three tests:

test_getBridges: sends an email to BridgeDB and checks that a response is received containing bridge details
test_getBridges_rateLimitExceeded: sends 3 emails to BridgeDB from the same email address and checks that the correct rate-limited response is received in each case
test_getBridges_stressTest: sends 100 emails from random email addresses on the same domain, and checks that a response is received in each case

To run the tests:

set EMAIL_SMTP_PORT in bridgedb.conf to 2525
add "127.0.0.1" to EMAIL_DOMAINS in bridgedb.conf
generate some bridge descriptors
run bridgedb
run 'trial test_smtp.py' from within the bridgedb virtualenv

Last edited 6 years ago by trygve (previous) (diff)

Changed 6 years ago by trygve

Attachment: test_https.py added

BridgeDB HTTPS integration test suite (twisted.trial)

comment:12 Changed 6 years ago by trygve

Attached 'test_https.py'. This is a test suite used to test the BridgeDB HTTPS web page. All tests are written to use twisted.trial. The suite currently has three tests:

test_get_obfs2_ipv4: uses mechanize and Beautiful soup to connect to the BridgeDB web page, follows some links, chooses 'obfs2' on the options page, enters the captcha and checks that at least one bridge line is returned of type 'obfs2'
test_get_obfs3_ipv4: as above, except gets 'obfs3' bridges
test_get_obfs3_ipv6: as above, except gets 'obfs3' ipv6 bridges. Note: currently fails on my system due to lack of ipv6 bridge descriptors available.

To run the tests:

generate some bridge descriptors (should contain at least one obfs2 and obfs3 bridges)
run bridgedb
run 'trial test_https.py' from within the bridgedb virtualenv

Last edited 6 years ago by trygve (previous) (diff)

comment:13 in reply to:  11 Changed 6 years ago by chingucha

Replying to trygve:

Attached 'test_smtp.py'. This is a test suite used to test the BridgeDB SMTP autoresponder. All tests are written to use twister.trial.

Why use threading + asyncore? Just use twisted to do the asyncy foo (twisted.mail.smtp)

comment:14 in reply to:  12 ; Changed 6 years ago by isis

Cc: sysrqb trygve added
Status: needs_reviewneeds_revision

Replying to trygve:

Attached 'test_https.py'. This is a test suite used to test the BridgeDB HTTPS web page. All tests are written to use twisted.trial. The suite currently has three tests:

test_get_obfs2_ipv4: uses mechanize and Beautiful soup to connect to the BridgeDB web page, follows some links, chooses 'obfs2' on the options page, enters the captcha and checks that at least one bridge line is returned of type 'obfs2'
test_get_obfs3_ipv4: as above, except gets 'obfs3' bridges
test_get_obfs3_ipv6: as above, except gets 'obfs3' ipv6 bridges. Note: currently fails on my system due to lack of ipv6 bridge descriptors available.

To run the tests:

generate some bridge descriptors (should contain at least one obfs2 and obfs3 bridges)
run bridgedb
run 'trial test_https.py' from within the bridgedb virtualenv


I had to do a bit of refactoring (not of your code, but of our CI scripts) to get this to work. All the work is in my fix/9874-https branch, including your script.

Previously, we had tests in lib/bridgedb/test/test_bridgedb.py which started a BridgeDB instance (including copying a bunch of configs and other files around). Now the BridgeDB instance is started before the CI infrastructure runs the tests and the old test_bridgedb.py tests were refactored to use the already running instance.

Your tests pass, all except the one test_get_obfs3_ipv6 test. This is because BridgeDB learns about a bridge's pluggable transports from the transport lines in @type bridge-extrainfo descriptors, and obfs3 (as with most PTs) cannot currently listen on an IPv6 address (#7961, #11211, #12138).

You could change the test_get_obfs3_ipv6 test to check for scramblesuit bridges on IPv4 if you like, but you'd need to patch leekspin to add "transport scramblesuit" lines. Or simply remove the failing test, I'll accept either.

Thanks, trygve, for all this work!


By the way, I have attributed you in git as trygve <tor-dev@lists.torproject.org>, please let me know if I should change the attribution.

comment:15 Changed 6 years ago by isis

Also, this is just the review of test_https.py, I haven't gotten to test_smtp.py yet. In retrospect, these probably should've been separate tickets, but oh well, moving on.

comment:16 in reply to:  14 Changed 6 years ago by isis

Replying to isis:

You could change the test_get_obfs3_ipv6 test to check for scramblesuit bridges on IPv4 if you like, but you'd need to patch leekspin to add "transport scramblesuit" lines. Or simply remove the failing test, I'll accept either.


You have transport scramblesuit lines now with leekspin==0.2.0, and the version has been bumped in BridgeDB. Also, it occurs to me that you could check IPv6 bridges for plain vanilla bridges (without any PTs).

comment:17 in reply to:  11 Changed 6 years ago by isis

Replying to trygve:

Attached 'test_smtp.py'. This is a test suite used to test the BridgeDB SMTP autoresponder. All tests are written to use twister.trial. The suite currently has three tests:

test_getBridges: sends an email to BridgeDB and checks that a response is received containing bridge details
test_getBridges_rateLimitExceeded: sends 3 emails to BridgeDB from the same email address and checks that the correct rate-limited response is received in each case
test_getBridges_stressTest: sends 100 emails from random email addresses on the same domain, and checks that a response is received in each case

To run the tests:

set EMAIL_SMTP_PORT in bridgedb.conf to 2525
add "127.0.0.1" to EMAIL_DOMAINS in bridgedb.conf
generate some bridge descriptors
run bridgedb
run 'trial test_smtp.py' from within the bridgedb virtualenv


The changes for your test_smtp.py script are in my fix/9874-email branch. All your tests pass (at least the test_smtp.py ones do).

I think that using asynccore, threading, and smtplib, while seemingly odd when Twisted provides this, is the right way to go for these tests. Twisted.trial tests which depend upon real networking are highly prone to difficult-to-debug problems with Twisted's reactor getting dirty, etc. Not to mention that using twisted.mail.smtp to essentially test the usage of twisted.mail.smtp in bridgedb.email seems... I'm not sure what the word is... like a truism? As in, the quirks in twisted.mail.smtp probably won't show up if we use twisted.mail.smtp to test it.

So, this script look fine to me. It depends on the extra changes I made in the fix/9874-https branch mentioned above, so that one test_get_obfs3_ipv6 unittest will still need to be fixed and I'll merge everything.

Thanks again!

Changed 6 years ago by trygve

Attachment: 20140807_test_https.patch added

Fixed failing ipv6 test (replaces with vanilla bridge), adds scrameblesuit test

comment:18 Changed 6 years ago by trygve

Status: needs_revisionneeds_review

Thank you for feedback. I've attached a patch to fix the failing test. I'm new to git (used to cvs, svn and hg), so apologies if I've done this wrong.

The ipv6 bridge has been changed to a vanilla ipv4 bridge. I've also added a new test to check for a scramblesuit bridge. Both of these surprised me because they had a different number of fields in each bridge line, but I've adjusted the tests accordingly.

A few comments:

  • mechanize is in '.test.requirements.txt' but not 'requirements.txt'. I'm assuming this is intentional, but it means that the https tests fail when using 'requirements.txt'
  • leakspin is in '.test.requirements.txt', but its dependency 'ipaddr' is not
  • I had to change EMAIL_SMTP_PORT to 2525 in bridgedb.conf. This might not be obvious to others.
  • I had to add "127.0.0.1" to EMAIL_DOMAINS in bridgedb.conf. This might not be obvious to others. It may be possible to change the scripts to send from one of the 3 allowed domains, as the SMTP server in the test script should intercept these emails without forwarding on to the real domains.
  • I had to perform a few more steps to get everything to work ('leakspin -n 100' and 'scripts/make-ssl-cert'). This was all documented in the excellent README, but would it be useful to create a little script that sets up the test environment automatically? You've added something that does this for Travis CI, but I've been working from the shell, entering commands manually.

comment:19 in reply to:  18 ; Changed 6 years ago by isis

Replying to trygve:

Thank you for feedback. I've attached a patch to fix the failing test. I'm new to git (used to cvs, svn and hg), so apologies if I've done this wrong.


Nope, it's cool. git checkout -b YOURBRANCH develop and then linking to a publicly available remote is generally the best way to go, mostly because straight patch files obviously lack commit metadata. I think there is a way to use git-hg and hg-git to work on a git repo in mercurial, but then I'm not super familiar with Mercurial.

The ipv6 bridge has been changed to a vanilla ipv4 bridge. I've also added a new test to check for a scramblesuit bridge. Both of these surprised me because they had a different number of fields in each bridge line, but I've adjusted the tests accordingly.


Yep! There's actually a possibly infinite (sort of) amount of k=vs which can be on the end of bridge-extrainfo transport lines, which BridgeDB is supposed to put on the end of a bridge line.

A few comments:

  • mechanize is in '.test.requirements.txt' but not 'requirements.txt'. I'm assuming this is intentional, but it means that the https tests fail when using 'requirements.txt'


I actually need to remove requirements.txt, or remove at least the part in the setup.py script which reads requirements.txt and parses everything into setuptools/distribute dependencies. On the production server, we actually delete requirements.txt before installing, because otherwise setuptools/distribute drops down to the embedded easy_install, which we believe is quite unsafe.

  • leakspin is in '.test.requirements.txt', but its dependency 'ipaddr' is not


When pip installs dependencies, it looks into the setuptools/distribute dependencies listed in the install_requires and dependency_links parameters to the setuptools.setup() call, and also in the MANIFEST.in file, if one is present. So pip recursively downloads dependencies. Also ipaddr is specified with a rather strange/special syntax in BridgeDB's requirements.txt:

https://ipaddr-py.googlecode.com/files/ipaddr-2.1.10.tar.gz#sha1=c608450b077b19773d4f1b5f1ef88b26f6650ce0#egg=ipaddr-2.1.10-py2.7

because BridgeDB requires a specific version of ipaddr (due to upstream bugs, which I've pointed out to the maintainers). This version is

  1. missing from PyPI (although other versions are there)
  1. https://googlecode.com is what pip calls "an external index" (meaning that it is not https://pypi.org). I filed several CVEs against pip last year for unsafe behaviours while crawling indexes, and they very responsibly fixed these issues. (In fact, it was already fixed in alpha versions of pip at the time, just not the version in all the distros.) Because we need to use "an external index" for this dependency, we specify https:// and the expected SHA-1 hash for the package, so that pip knows how to verify it.


  • I had to change EMAIL_SMTP_PORT to 2525 in bridgedb.conf. This might not be obvious to others.
  • I had to add "127.0.0.1" to EMAIL_DOMAINS in bridgedb.conf. This might not be obvious to others. It may be possible to change the scripts to send from one of the 3 allowed domains, as the SMTP server in the test script should intercept these emails without forwarding on to the real domains.


This is possible, and I considered doing it, but I didn't really want to change your scripts while testing them out. It requires a change to /etc/hosts, however, which we already do for Travis, but it's perhaps less nice to ask other people to do this in order to run the tests.

We could change your test_smtp.py script to use example.com, set that in /etc/hosts for Travis, and then have your tests skip themselves if they aren't being run on Travis. Or...as you suggest:

  • I had to perform a few more steps to get everything to work ('leakspin -n 100' and 'scripts/make-ssl-cert'). This was all documented in the excellent README, but would it be useful to create a little script that sets up the test environment automatically? You've added something that does this for Travis CI, but I've been working from the shell, entering commands manually.


We could move the before_install section in .travis.yml to some script, but then we need to tell people to run a script which runs more scripts before testing. The nice thing about having the command separate in Travis is that you can see which one failed, versus a whole setup script failing. I'm not really sure which one is better. Unless you have more ideas?

It occurs to me that your tests might fail on random machines, like if port 2525 was already in use, or they have a weird firewall configuration. Perhaps we should only expect them to run in special environments?

We could do export TESTING_BRIDGEDB=1 in the test environment setup script and Travis, and then check for that in your tests, rather than checking for os.env.get("TRAVIS") and only running on Travis. That way, your tests would only run if the test environment setup script had been run, or on CI machines, but would skip if someone tried to run all the tests on a random machine.

Or we could just add the sed commands to the README and wish everyone the best of luck getting this crazy program to run. :/

Changed 6 years ago by trygve

comment:20 in reply to:  19 Changed 6 years ago by trygve

Replying to isis:

Replying to trygve:

Thank you for feedback. I've attached a patch to fix the failing test. I'm new to git (used to cvs, svn and hg), so apologies if I've done this wrong.


Nope, it's cool. git checkout -b YOURBRANCH develop and then linking to a publicly available remote is generally the best way to go, mostly because straight patch files obviously lack commit metadata. I think there is a way to use git-hg and hg-git to work on a git repo in mercurial, but then I'm not super familiar with Mercurial.

The writing is on the wall for hg, so I'm happy to learn git. I'm probably going to make a lot of rookie mistakes, so please bear with me. I've used git format-patch to create a patch with the commit meta data, I hope that will be ok? All changes were made from your fix/9874-email branch

  • I had to perform a few more steps to get everything to work ('leakspin -n 100' and 'scripts/make-ssl-cert'). This was all documented in the excellent README, but would it be useful to create a little script that sets up the test environment automatically? You've added something that does this for Travis CI, but I've been working from the shell, entering commands manually.


We could move the before_install section in .travis.yml to some script, but then we need to tell people to run a script which runs more scripts before testing. The nice thing about having the command separate in Travis is that you can see which one failed, versus a whole setup script failing. I'm not really sure which one is better. Unless you have more ideas?

I don't think I know enough about Travis to answer that intelligently ;)


It occurs to me that your tests might fail on random machines, like if port 2525 was already in use, or they have a weird firewall configuration. Perhaps we should only expect them to run in special environments?

Port 2525 already in use should cause the tests to fail with an 'Address already in use' (or whatever) exception, which should be easy to debug. Not so much if iptables is doing '-j DROP'. I'm not sure how to mitigate this, other than by adding some comments to the README and the tests themselves.


We could do export TESTING_BRIDGEDB=1 in the test environment setup script and Travis, and then check for that in your tests, rather than checking for os.env.get("TRAVIS") and only running on Travis. That way, your tests would only run if the test environment setup script had been run, or on CI machines, but would skip if someone tried to run all the tests on a random machine.

Or we could just add the sed commands to the README and wish everyone the best of luck getting this crazy program to run. :/

I guess what we're trying to avoid is unexplained, hard-to-debug failures when people run the unit tests. If you agree, I can submit a patch which does the following:

  • add a few comments to the end of the README
  • update test_smtp.py and test_https.py to raise SkipTest if the TESTING_BRIDGEDB (or TRAVIS?) environment variable is not set
  • find a way to make test_https.py behave if mechanize is not installed (it's in .test.requirements.txt, but not requirements.txt). Raising SkipTest isn't possible at the moment because the import occurs at the top of the file before any tests have been run. I can probably defer the import until the test setup() function, and raise SkipTest if importing it fails
  • update test_smtp.py and test_https.py to raise SkipTest if bridgedb is not running (like what test_bridgedb.py does)
  • update .test.requirements.txt to include ipaddr (the one hosted on googlecode.com)
  • add a setup_test_environment.sh (or whatever) bash script which sets up the test environment i.e. uses sed to modify bridgedb.conf, installs .test.requirements, runs leekspin, generates ssl certs and sets the required environment variable. Actually I'm not sure about that last bit: the script can set the variable in its own environment, but it can't affect the parent's environment, so as soon as the script returns the variable will be lost. How would you expect these to tests to actually be executed? When bridgedb test is run, or using some other method e.g. directly from the shell? Adding a new command line parameter to bridgedb? Adding a 2nd run_integration_tests.sh script? I guess I'm missing the bigger picture as to how other people may use this (if at all). I don't want to suggest making invasive or unnecessary changes unless you think there's a good reason (or no alternative)

comment:21 Changed 6 years ago by isis

I've merged the fix/9874-email and fix/9874-https branches into develop.

Changed 6 years ago by trygve

Fixes to test_https.py

Changed 6 years ago by trygve

Fixes to test_smtp.py

Changed 6 years ago by trygve

Adds ipaddr to .test.requirements.txt

comment:22 Changed 6 years ago by trygve

Added three new small patches:

  • 0001: Raise SkipTest if TESTING_BRIDGEDB environment variable not set in test_https.py. Also fixes bug in bridge-line parsing function which would fail if more than one bridge was present.
  • 0002: Raise SkipTest if TESTING_BRIDGEDB environment variable not set in test_smtp.py
  • 0003: ipaddr was in requirements.txt but not .test.requirements.txt

I'm sure you want to see the back of this ticket, but only that first one is important (I probably should have split it into two separate patches)

comment:23 in reply to:  22 ; Changed 6 years ago by isis

Replying to trygve:

Added three new small patches:

  • 0001: Raise SkipTest if TESTING_BRIDGEDB environment variable not set in test_https.py. Also fixes bug in bridge-line parsing function which would fail if more than one bridge was present.


As you've probably already noticed, BridgeDB has some logic to hand out different amounts of bridge lines depending on the number available (as you've probably noticed). It was causing some intermittent test failures in other later development branches, so I went ahead and fixed it before you posted this. :/ I'm sorry! I didn't realise you were already on it.

For the TESTING_BRIDGEDB envvar, I'd kind of rather see this done by testing for a running PID, as how it's done here in lib/bridgedb/test/test_bridgedb.py.

  • 0002: Raise SkipTest if TESTING_BRIDGEDB environment variable not set in test_smtp.py


Same as above.

  • 0003: ipaddr was in requirements.txt but not .test.requirements.txt


When Leekspin is installed as a dependency, it should pull in it's own dependencies by itself. The .test.requirements.txt file was meant to only be the extra dependencies needed to run BridgeDB's test suites, on top of the other ones already needed just to run BridgeDB. The ipaddr module is required to run BridgeDB.

Did ipaddr somehow not get installed when you set up a BridgeDB instance?

I'm sure you want to see the back of this ticket, but only that first one is important (I probably should have split it into two separate patches)


No worries. If you want to revise this to use the PID, I'd still take #1 and #2!

comment:24 in reply to:  23 Changed 6 years ago by isis

Keywords: bridgedb-0.2.4 added
Resolution: fixed
Status: needs_reviewclosed

Replying to isis:

For the TESTING_BRIDGEDB envvar, I'd kind of rather see this done by testing for a running PID, as how it's done here in lib/bridgedb/test/test_bridgedb.py.


Since all this needed was copy/pasting code from one test suite to two others, and it was causing so many irrelevant tracebacks in my local test runs for other branches I'm testing, that it drove me nuts and I went ahead and committed it.

Sorry for jumping ahead! In retrospect, I probably should have given you more time to get everything in your tests just right before I merged anything. Sorry again.

And thanks again, trygve, for all your help on this!

Note: See TracTickets for help on using tickets.