Merge lp:~mandel/ubuntuone-dev-tools/proxy-testcase into lp:ubuntuone-dev-tools
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Approved by: | dobey on 2012-01-06 | ||||
| Approved revision: | 83 | ||||
| Merged at revision: | 52 | ||||
| Proposed branch: | lp:~mandel/ubuntuone-dev-tools/proxy-testcase | ||||
| Merge into: | lp:ubuntuone-dev-tools | ||||
| Diff against target: |
1215 lines (+1098/-25) 10 files modified
data/squid.conf.in (+120/-0) ubuntuone/devtools/services/__init__.py (+53/-0) ubuntuone/devtools/services/dbus.py (+8/-23) ubuntuone/devtools/services/squid.py (+245/-0) ubuntuone/devtools/services/tests/test_dbus.py (+1/-1) ubuntuone/devtools/services/tests/test_squid.py (+334/-0) ubuntuone/devtools/testcases/dbus.py (+1/-1) ubuntuone/devtools/testcases/squid.py (+58/-0) ubuntuone/devtools/testcases/tests/__init__.py (+16/-0) ubuntuone/devtools/testcases/tests/test_squid_testcase.py (+262/-0) |
||||
| To merge this branch: | bzr merge lp:~mandel/ubuntuone-dev-tools/proxy-testcase | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| dobey (community) | 2011-12-08 | Approve on 2012-01-05 | |
| Alejandro J. Cura (community) | Approve on 2012-01-04 | ||
|
Review via email:
|
|||
Commit Message
- A SquidTestCase that does the following:
- Creates a new password file.
- Creates a squid.conf
- Launches squid for tests listening to two random ports, one with auth and other without.
- Tests for all the above.
- Cleans a little the dbus testcase.
Description of the Change
This branch adds the following:
- A SquidTestCase that does the following:
- Creates a new password file.
- Creates a squid.conf
- Launches squid for tests listening to two random ports, one with auth and other without.
- Tests for all the above.
- Cleans a little the dbus testcase.
| dobey (dobey) wrote : | # |
There's a conflict now with trunk. Please merge trunk and fix the conflict, as well as changing the remaining usage of xdg.BaseDirectory in your branch to use dirspec.basedir instead. The dirspec code is packaged in the nightlies PPA now, and is cross-platform and based on the code in ubuntu_sso and xdg.
Also, please fix the rest of the previously mentioned issues.
| Alejandro J. Cura (alecu) wrote : | # |
Lovely and well tested branch!
A few small comments:
The first line of this should not be needed:
541 + template = None
542 + with open(path) as in_file:
543 + template = string.
Regarding this:
return dict(address=
Can the key "address" be changed to "host"? I think it's a better name, and it matches other parts.
(PS: I know I might have suggested that name in the first place, sorry about that, but I think it's easier to change now than later)
| dobey (dobey) wrote : | # |
So, it seems like we can combine the 2 squid config files. I copied the squid3.conf.in over top of squid2.conf.in, and the tests still pass for me, on Oneiric at least. The diff shows a small set of changes, most of which the older squid possibly just ignores.
>Starting squid version 2...
>squid: ERROR: Could not send signal 0 to process 32571: (3) No such process
>Waiting for squid to start..
This error happens with both versions of squid, both on Oneiric and Precise for me. Do we know why this is happening, and how we can get rid of it?
On Precise, I also get the following:
WARNING: cache_mem is larger than total disk cache space!
WARNING: (B) '::/0' is a subnetwork of (A) '::/0'
WARNING: because of this '::/0' is ignored to keep splay tree searching predictable
WARNING: You should probably remove '::/0' from the ACL named 'all'
Is there any way to get rid of the first warning, via the config? Perhaps setting a value of cache_mem to be quite small?
For the 'all' ACL issue, can we redirect the STDOUT/STDERR output from squid, to a log file (or two) in the tempdir?
| Manuel de la Peña (mandel) wrote : | # |
> So, it seems like we can combine the 2 squid config files. I copied the
> squid3.conf.in over top of squid2.conf.in, and the tests still pass for me, on
> Oneiric at least. The diff shows a small set of changes, most of which the
> older squid possibly just ignores.
>
> >Starting squid version 2...
> >squid: ERROR: Could not send signal 0 to process 32571: (3) No such process
> >Waiting for squid to start..
That occurs because I'm trying to talk with squid to ensure that is running before we start the the test case, mainly to ensure that there are no race condition. I can forward the stderr to a log file so that we see if happens but does not leave the terminal buffer full of junk.
>
> This error happens with both versions of squid, both on Oneiric and Precise
> for me. Do we know why this is happening, and how we can get rid of it?
>
> On Precise, I also get the following:
>
> WARNING: cache_mem is larger than total disk cache space!
>
> WARNING: (B) '::/0' is a subnetwork of (A) '::/0'
> WARNING: because of this '::/0' is ignored to keep splay tree searching
> predictable
> WARNING: You should probably remove '::/0' from the ACL named 'all'
>
> Is there any way to get rid of the first warning, via the config? Perhaps
> setting a value of cache_mem to be quite small?
> For the 'all' ACL issue, can we redirect the STDOUT/STDERR output from squid,
> to a log file (or two) in the tempdir?
I'll play around with the settings to see if I can solve the issues.
| Manuel de la Peña (mandel) wrote : | # |
Config and code have been updated to remove the errors. It was quite simple.
| dobey (dobey) wrote : | # |
With the fix to redirect the warnings and errors, we can keep the "acl all src all" in squid3.conf.in, and use it for both versions, it seems. I added it back, and copied squid3.conf.in to squid2.conf.in, and the tests work in both Precise and Oneiric for me, though squid3 does seem to take a bit long to start for some reason, even without that line in the config. Can we add this config back, and just have a single squid.conf.in for both versions?
Also, can you change the service code to look for squid3 first, rather than squid? It's better to try the new first, then fall back to the old version, I think.
| Manuel de la Peña (mandel) wrote : | # |
> With the fix to redirect the warnings and errors, we can keep the "acl all src
> all" in squid3.conf.in, and use it for both versions, it seems. I added it
> back, and copied squid3.conf.in to squid2.conf.in, and the tests work in both
> Precise and Oneiric for me, though squid3 does seem to take a bit long to
> start for some reason, even without that line in the config. Can we add this
> config back, and just have a single squid.conf.in for both versions?
>
Done
> Also, can you change the service code to look for squid3 first, rather than
> squid? It's better to try the new first, then fall back to the old version, I
> think.
Done
| dobey (dobey) wrote : | # |
Looks good now, though the comment is wrong for the block of code that finds the squid executable. :)
| dobey (dobey) wrote : | # |
Manuel, please add the shortened e-mail address used in your recent commits, to your LP account, so that LP knows who you are. :)
| Ubuntu One Auto Pilot (otto-pilot) wrote : | # |
The attempt to merge lp:~mandel/ubuntuone-dev-tools/proxy-testcase into lp:ubuntuone-dev-tools failed. Below is the output from the failed tests.
ubuntuone.
BaseTestCase
runTest ... [OK]
ubuntuone.
EnvironTestCase
test_
test_
test_
PathsTestCase
test_
test_
test_
test_
test_
test_
test_
SquidRunnerIn
test_
test_
SquidRunnerTe
test_
test_
test_
ubuntuone.
DBusTestCase
runTest ... [OK]
ubuntuone.
TestWithDBus
test_
test_
ubuntuone.
TestCheckTwis
test_
test_bare_super ... [OK]
test_
test_
test_
test_
test_
test_
test_
TestTwistedCh
test_
test_
twisted.
TestCase
runTest ... [OK]
ubuntuone.
BaseTestCase
runTest ... [OK]
ubuntuone.
Tes...

856 +"""Base swuid tests cases and test utilities."""
swuid -> squid
861 +# DBusRunner for DBusTestCase using tests devtools. services. squid import (
862 +from ubuntuone.
I don't think it's importing DBus bits here. :)
151 +def find_config_ file(in_ config_ file, tempdir=None):
find_config_file doesn't actually even use tempdir, so you can remove the argument, and change the calls to it, to not pass it in.