Feature #6305: Write tests for Tor bridges

Feature #6305

Write tests for Tor bridges

Added by bertagaz about 12 years ago. Updated about 10 years ago.

Status:

Resolved

Priority:

Normal

Assignee:

Category:

Test suite

Target version:

Tails_1.3

Start date:

2013-09-26

Due date:

% Done:

100%

Feature Branch:

test/6305-tor-bridges

Type of work:

Code

Blueprint:

Starter:

Affected tool:

Deliverable for:

Description

Write tests to ensure that the bridge functionnality is well supported in Tails.

Subtasks

Related issues

Blocked by Tails - ~~Feature #7821~~: Automatic tests for Tor

Resolved

2014-08-26

History

#1 Updated by intrigeri about 12 years ago

Target version set to Sustainability_M1

#2 Updated by intrigeri about 12 years ago

Subject changed from Write bridges tests to Write tests for Tor bridges

#3 Updated by intrigeri about 11 years ago

Parent task changed from ~~Feature #6298~~ to ~~Feature #6839~~

#4 Updated by intrigeri about 11 years ago

related to deleted (~~~~Feature #6298~~: Write more automated tests~~)

#5 Updated by intrigeri about 11 years ago

related to ~~Feature #7821~~: Automatic tests for Tor added

#6 Updated by intrigeri about 11 years ago

Assignee set to anonym
Target version changed from Sustainability_M1 to Tails_1.2.2

#7 Updated by sajolida about 11 years ago

blocked by #8117 added

#8 Updated by sajolida about 11 years ago

blocks deleted (~~#8117~~)

#9 Updated by sajolida about 11 years ago

blocks #8117 added

#10 Updated by anonym about 11 years ago

Status changed from Confirmed to In Progress
% Done changed from 0 to 30
Feature Branch set to test/6305-tor-bridges

Did some minimal work on this today, which actually may be enough:

While it only tests obfs3 bridges, I think that’s enough (until we support scramblesuit, i.e. ~~Feature #7909~~), since “vanilla” and obfs2 bridges are obsolete. Adding scenarios for them would be little more than a trivial copy-paste away, but IMHO that’d only make the test suite run slower.

Also, in the manual test suite’s bridge section we have the vague “Use the Internet” step, to generate some network activity. The current tests only activity is the actual Tor bootstrap and the upgrade check. If that isn’t enough I suppose can, quite arbitrarily, throw in a “check that Tails’ website loads in the Tor Browser” step too.

#11 Updated by intrigeri about 11 years ago

anonym wrote:
> Did some minimal work on this today, which actually may be enough:

Congrats!

> * While it only tests obfs3 bridges, I think that’s enough (until we support scramblesuit, i.e. ~~Feature #7909~~), since “vanilla” and obfs2 bridges are obsolete. Adding scenarios for them would be little more than a trivial copy-paste away, but IMHO that’d only make the test suite run slower.

I don’t see regular bridges as obsolete (people for whom they work should not consume resources from the smaller PT bridge pools IMO), and I see value in testing both bridges that are handled by Tor itself in addition to bridges that are managed by PTs.

> * Also, in the manual test suite’s bridge section we have the vague “Use the Internet” step, to generate some network activity. The current tests only activity is the actual Tor bootstrap and the upgrade check.

Good enough.

#12 Updated by anonym about 11 years ago

Target version changed from Tails_1.2.2 to Tails_1.2.3

#13 Updated by anonym about 11 years ago

Target version changed from Tails_1.2.3 to Tails_1.3

#14 Updated by Tails about 10 years ago

Applied in changeset commit:2c420ac45ea9a92d4fec5b81071adbfbcc14d539.

#15 Updated by anonym about 10 years ago

related to deleted (~~~~Feature #7821~~: Automatic tests for Tor~~)

#16 Updated by anonym about 10 years ago

blocked by ~~Feature #7821~~: Automatic tests for Tor added

#17 Updated by anonym about 10 years ago

Assignee changed from anonym to kytv
% Done changed from 30 to 50
QA Check set to Ready for QA

Forced pushed a completely re-worked version of the branch. It’s based on test/7821-tor, which isn’t merged as of writing this, so I added a blocker to ~~Feature #7821~~. I’d still appreciate an initial code review even before test/7821-tor is merged, is convenient.

#18 Updated by anonym about 10 years ago

anonym wrote:
> I’d still appreciate an initial code review even before test/7821-tor is merged, is convenient.

Tip for this:

git log --cc --reverse -p  origin/testing..origin/test/6305-tor-bridges ^origin/test/7821-tor

#19 Updated by kytv about 10 years ago

Initial comments:

One problem, so far, perhaps with wiki/src/contribute/release_process/test/usage.mdwn or features/step_definitions/tor.rb.

Running the time_syncing.feature failed with

 Scenario: Clock with host's time in bridge mode                        # features/time_syncing.feature:54
    When the network is plugged                                          # features/step_definitions/common_steps.rb:112
    And the Tor Launcher autostarts                                      # features/step_definitions/tor.rb:322
    And I configure some Bridge pluggable transports in Tor Launcher     # features/step_definitions/tor.rb:327
      uninitialized constant AssertionFailedError (NameError)
      ./features/step_definitions/tor.rb:335:in `rescue in block in <top (required)>'
      ./features/step_definitions/tor.rb:331:in `/^I configure some (\w+) pluggable transports in Tor Launcher$/'
      features/time_syncing.feature:57:in `And I configure some Bridge pluggable transports in Tor Launcher'
    And Tor is ready                                                     # features/step_definitions/common_steps.rb:306
    Then Tails clock is less than 5 minutes incorrect                    # features/step_definitions/time_syncing.rb:35

My features/config/local.yml contained lines like

Tor:
  Transports:
    bridge:
      - ipv4_address: "xxx.yyy.zzz.aaa"
        ipv4_port: xxxx
        fingerprint: "SOMEFINGERPRINT"
        extra:
 .....

and wiki/src/contribute/release_process/test/usage.mdwn contains

where the type `$TYPE` (and `$ANOTHER_TYPE`) should be something like
`obfs4` or `bridge` (the first type) or whatever Tor calls them. ...

When I changed the yml file to contain Bridge the test proceeded.

To be continued…

#20 Updated by kytv about 10 years ago

Assignee changed from kytv to anonym
QA Check changed from Ready for QA to Dev Needed

The only problem that I saw was with respect to the config file handling (and maybe the documentation).

If we want to mandate that the bridge_type is upcase!’d and the config file specifies obfs3 I should see the warning

It seems no '#{bridge_type}' pluggable transports are defined in your local configuration file

but instead I saw

uninitialized constant AssertionFailedError (NameError)

When bridge_type is in the correct format in the yaml file the tests—AFAICT—test what should be tested.

If bridge_type should be specified with a leading capital letter:

diff --git a/wiki/src/contribute/release_process/test/usage.mdwn b/wiki/src/contribute/release_process/test/usage.mdwn
index 813d677..9376cee 100644
--- a/wiki/src/contribute/release_process/test/usage.mdwn
+++ b/wiki/src/contribute/release_process/test/usage.mdwn
@@ -103,12 +103,12 @@ The format is:
           [...]

 where the type `$TYPE` (and `$ANOTHER_TYPE`) should be something like
-`obfs4` or `bridge` (the first type) or whatever Tor calls them. Both
+`Obfs4` or `Bridge` (the first type) or whatever Tor calls them. Both
 `fingerprint` and `extra` are optional and can be left empty (or
-skipped completely), but e.g. `extra` is necessary for `obfs4` type
+skipped completely), but e.g. `extra` is necessary for `Obfs4` type
 bridges, for the `cert=... iat-mode=...` stuff, and the same for
-`scramblesuite`'s `password=...`.
+`Scramblesuite`'s `password=...`.

 This setting is required for `tor_bridges.feature` (requires types
-`bridge`, `obfs2`, `obfs3` and `obfs4`) and `time_syncing.feature`
-(requires type `bridge` only).
+`Bridge`, `Obfs2`, `Obfs3` and `Obfs4`) and `time_syncing.feature`
+(requires type `Bridge` only).

I’m not sure that’s desired but I can see how it’d make sense to have the scalars capitalized! with the sequences downcased!.

#21 Updated by anonym about 10 years ago

Assignee changed from anonym to kytv
QA Check changed from Dev Needed to Ready for QA

kytv wrote:
> @uninitialized constant AssertionFailedError (NameError)|

Oops, now fixed.

> If bridge_type should be specified with a leading capital letter:
>
> […]

Thanks for the patch! Now applied.

#22 Updated by intrigeri about 10 years ago

Code review passes. Not tested.

#23 Updated by Tails about 10 years ago

Status changed from In Progress to Fix committed
% Done changed from 50 to 100

Applied in changeset commit:4bb35fc063a9fe695a3da7df1062b11d9fc50157.

#24 Updated by intrigeri about 10 years ago

Assignee deleted (~~kytv~~)
QA Check changed from Ready for QA to Pass

#25 Updated by BitingBird about 10 years ago

Status changed from Fix committed to Resolved