Feature #9129

Write regression tests for installing Tails to too small devices

Added by anonym 2015-03-30 08:43:26 . Updated 2015-05-12 18:41:30 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Test suite
Target version:
Start date:
2015-03-30
Due date:
% Done:

100%

Feature Branch:
kytv:test/9129-tails-installer-small-devices
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Installer
Deliverable for:

Description

Regression test for Bug #6538.


Subtasks


Related issues

Related to Tails - Bug #6538: Tails Installer tries to install to too small devices Resolved 2013-12-26
Blocked by Tails - Feature #9131: Write regressions tests for updated device list in Tails installer Resolved 2015-03-30

History

#1 Updated by intrigeri 2015-03-30 08:55:16

#2 Updated by intrigeri 2015-03-30 08:55:36

  • blocks #8538 added

#3 Updated by intrigeri 2015-03-30 08:55:50

  • related to Bug #6538: Tails Installer tries to install to too small devices added

#4 Updated by anonym 2015-03-30 11:39:31

The following commits in the bugfix/8691-and-6538 branch contains some WIP for this:

  • 9a96aae814416a0370324c818b307e1628a96c54
  • ee4ebdff1b3d0615f6797772a68d046cdc2748a4

#5 Updated by anonym 2015-03-30 11:39:58

  • Tracker changed from Bug to Feature

#6 Updated by kytv 2015-04-09 23:19:46

  • Status changed from Confirmed to In Progress
  • QA Check set to Dev Needed

I’m looking at this now but it seems that I’ll need to wait for Bug #9130 in order to properly finish this since with drives that are too small Tails Installer says that no device is found.

#7 Updated by kytv 2015-04-12 17:35:45

  • Assignee changed from kytv to intrigeri
  • % Done changed from 0 to 30
  • QA Check changed from Dev Needed to Ready for QA
  • Feature Branch set to kytv:test/9129-tails-installer-small-devices

The test works now but will need to be adjusted once Bug #9130 is finished.

#8 Updated by kytv 2015-04-12 17:36:11

  • blocked by Feature #9131: Write regressions tests for updated device list in Tails installer added

#9 Updated by intrigeri 2015-04-14 12:43:31

  • Assignee changed from intrigeri to anonym

#10 Updated by anonym 2015-04-15 12:43:54

  • Assignee changed from anonym to kytv
  • QA Check changed from Ready for QA to Dev Needed
  Scenario: Try installing Tails to a too small USB drive
[...]
    Then I plug USB drive "current"
    But a suitable USB device is still not found


I’m not sure I trust this part — I worry that we don’t explicitly wait for Tails Installer to detect (and drop) the device. The I plug ... step will wait until the device is detected (until test -b $DEVICE succeeds) but I imagine Tails Installer needs more time if it were to list the device.

The safest may be to start Tails Installer from the commandline, with the verbose flag, redirect it to some log file, and then poll it through the remote shell for the event when it’s plugged and make sure it’s logged that it is dropped. Then we also make sure that the device list is still empty do a suitable USB device is still not found.

#11 Updated by kytv 2015-04-16 17:49:23

anonym wrote:
> […]
> I’m not sure I trust this part — I worry that we don’t explicitly wait for Tails Installer to detect (and drop) the device. The I plug ... step will wait until the device is detected (until test -b $DEVICE succeeds) but I imagine Tails Installer needs more time if it were to list the device.

I did test with an older Tails and the test failed (as it should) but your concerns are still valid. To be addressed.

> The safest may be to start Tails Installer from the commandline, with the verbose flag, redirect it to some log file, and then poll it through the remote shell for the event when it’s plugged and make sure it’s logged that it is dropped. Then we also make sure that the device list is still empty do a suitable USB device is still not found.

This seems like a good way of handling it. I’ll give it a shot.

#12 Updated by kytv 2015-04-23 18:33:38

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

Hopefully the way that I did this is acceptable. As suggested, the test parses the output of liveusb-creator-launcher --verbose

  1. test fails with 1.3.1
  2. test passes with 1.3.2
  3. test passes with experimental

#13 Updated by anonym 2015-05-05 14:24:14

  • Assignee changed from anonym to kytv
  • % Done changed from 30 to 40
  • QA Check changed from Ready for QA to Dev Needed

Looks very good! I just have one further robustness improvement request:

+    @vm.file_content('/tmp/tails-installer.log').include?('Skipping too small device')

I’m not sure how (who knows what the future holds?), but what if there’s some other device present that’s discarded for being too small? Then we get a false positive. Let’s try to fail safe whenever possible! @vm.disk_dev("current") will produce the device path so we can match for the full log message (untested code):

--- a/features/step_definitions/usb.rb
+++ b/features/step_definitions/usb.rb
@@ -123,11 +123,13 @@ When /^I start Tails Installer in "([^"]+)" mode(?: with the )?(verbose)?(?: fla
   end
 end

-Then /^Tails Installer detects that the device is too small$/ do
+Then /^Tails Installer detects that device "[^"]*" is too small$/ do |name|
   next if @skip_steps_while_restoring_background
   assert(@vm.file_exist?('/tmp/tails-installer.log'), "Cannot find logfile containing output from Tails Installer")
+  device = @vm.disk_dev(name)
   try_for(15, :msg => "Tails Installer did not reject the USB device as being too small")  {
-    @vm.file_content('/tmp/tails-installer.log').include?('Skipping too small device')
+    log = @vm.file_content('/tmp/tails-installer.log')
+    assert_not_nil(log.match?(/Skipping too small device: #{device}$/))
   }
 end

diff --git a/features/usb_install.feature b/features/usb_install.feature
index 3ed2383..8e5d464 100644
--- a/features/usb_install.feature
+++ b/features/usb_install.feature
@@ -12,7 +12,7 @@ Feature: Installing Tails to a USB drive, upgrading it, and using persistence
     And I start Tails Installer in "Clone & Install" mode with the verbose flag
     But a suitable USB device is not found
     When I plug USB drive "current"
-    Then Tails Installer detects that the device is too small
+    Then Tails Installer detects that device "current" is too small
     And a suitable USB device is not found

   @keep_volumes

#14 Updated by kytv 2015-05-05 21:54:56

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

Your patch snippet was very useful. I couldn’t get it to work using match (as you did, but I tried other other variations too), so I went with the include? like I did previously.

The tests in the branch will (now) look for the device in the output.

Tested with

  • a 1.2.3 ISO (failed)
  • a 1.4~rc1 ISO (passed)

#15 Updated by anonym 2015-05-07 15:19:09

  • Assignee changed from anonym to kytv
  • % Done changed from 40 to 50
  • QA Check changed from Ready for QA to Dev Needed

kytv wrote:
> Your patch snippet was very useful. I couldn’t get it to work using match (as you did, but I tried other other variations too), so I went with the include? like I did previously.

Sorry, in my patch the ? in match? shouldn’t be there. Can you please try again with the regex + match? I really like the anchor on the end for this test to be robust, since the device numbers can caused an ambiguous situation (e.g. sda1 vs sda11… :)).

#16 Updated by kytv 2015-05-07 16:33:10

I think I might see why it didn’t work for me (I did remove the ? earlier). Testing.

#17 Updated by kytv 2015-05-07 17:01:48

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

Sigh. Obviously I should have escaped the / in the device name in the regex. THAT was why it didn’t work for me. It’s such an obvious thing but I missed it. :|

#18 Updated by kytv 2015-05-07 19:46:52

kytv wrote:
> Sigh. Obviously I should have escaped the / in the device name in the regex. THAT was why it didn’t work for me. It’s such an obvious thing but I missed it. :|

Or, as anonym advised on IRC, Regexp.new. Thanks. :)

I amended the last commit and force pushed.

#19 Updated by kytv 2015-05-08 14:19:10

Applied in changeset commit:41e2c9cecd89379b5902f8a3a7da452cb55819e8.

#20 Updated by anonym 2015-05-08 14:19:10

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

Applied in changeset commit:d7f35308c4ad56dd51d58cdd3e9e3b5d746cc795.

#21 Updated by anonym 2015-05-08 14:21:52

  • Assignee deleted (anonym)
  • QA Check changed from Ready for QA to Pass

#22 Updated by BitingBird 2015-05-12 18:41:31

  • Status changed from Fix committed to Resolved