Write automated tests for growing system partition
The time spent on this is to be equally shared between Cyril and intrigeri (mentoring).
The new code that requires additional testing is https://git.tails.boum.org/tails/tree/config/chroot_local-includes/usr/share/initramfs-tools/scripts/init-premount/partitioning?h=feature/15292-repartition
New tests that might be worth writing:
- the FAT filesystem on the system partition is at least 4096M large
- the label of the FAT filesystem on the system partition is “Tails”
- the system partition is an ESP
- the FS UUID for the system partition is not a69020d2 (i.e.
/dev/disk/by-uuid/A690-20D2does not exist)
- the system partition has the expected GPT attributes/flags (regression test for
Expected GPT attributes:
SYSTEM_PARTITION_FLAGS = ( 1 << 0 | # system partition 1 << 2 | # legacy BIOS bootable 1 << 60 | # read-only 1 << 62 | # hidden 1 << 63 # do not automount )
Blocked by Tails -
#5 Updated by CyrilBrulebois 2018-11-28 15:26:58
- Assignee changed from CyrilBrulebois to intrigeri
- QA Check set to Ready for QA
- Feature Branch set to test/16003-growing-system-partition
Here’s the new scenario:
Scenario: Starting Tails from an USB image, and making sure it is correctly set up and resized Given a computer And I temporarily create a 7200 MiB disk named "usbimage" And I write the Tails USB image to disk "usbimage" And I start Tails from USB drive "usbimage" with network unplugged and I login Then Tails is running from USB drive "usbimage" And the label of the FAT filesystem on the system partition on "usbimage" is "Tails" And the system partition on "usbimage" is an ESP And the FAT filesystem on the system partition on "usbimage" is at least 4096M large And the FS UUID for the system partition on "usbimage" was randomized
And I’ve managed to test both positive and negative results with an IMG file built from the 3.10.1 ISO (so without any repartition code), from the current feature branches for
Feature #15292, with or without the prospective workaround mentioned in Bug #16153.
#7 Updated by intrigeri 2018-11-28 17:39:40
- Assignee changed from intrigeri to CyrilBrulebois
- % Done changed from 0 to 30
- QA Check changed from Ready for QA to Dev Needed
- I think we need to adjust a few things now that we support
TAILS_IMG: look for
- In the “the FAT filesystem on the system partition” step, I’m not sure about using guestfs on the device while the system is booted. I trust you it works right now but it feels risky to rely on it. I’d rather query this info from the system under test (as you do for the filesystem size). Alternatively, let the system boot, then shut it down, then do all the partition and FS checks offline with guestfs.
- I’ve pushed a few commits where I explain why I’m changing things, as agreed elsewhere.
#8 Updated by intrigeri 2018-11-29 12:22:12
https://jenkins.tails.boum.org/view/Tails_ISO/job/test_Tails_ISO_test-16003-growing-system-partition/3/cucumberTestReport/ failed with
FAT filesystem is too small: 4286566400 is less than 4294967296. Pushed a fix for it so we benefit from this test and the following steps.
#10 Updated by intrigeri 2018-12-02 19:59:06
Also, “the label of the FAT filesystem on the system partition” checks the label of the partition (which is indeed “Tails”), not the label of the filesystem. Otherwise the test would fail: the label is actually “TAILS”. I think you can extract it from the
IdLabel field in the
org.freedesktop.UDisks2.Block section of the
udisksctl info --block-device /dev/sda1 output.
#14 Updated by CyrilBrulebois 2018-12-27 14:51:23
Hi u, I’ve been dealing with administrativia and therefore a bit away from hacking lately; I’ve resumed actual work a few days ago, and this is next on my list. If that’s fine with you: Let’s discuss further in one week if progress hasn’t happened by then?
#18 Updated by CyrilBrulebois 2019-01-04 16:59:55
After discussion with intrigeri, we decided to have both label checks: one for the partition (Tails) and one for the FS (TAILS).
Just pushed this accordingly:
c8fcdafd5d..48bd529ad0 HEAD -> test/16003-growing-system-partition
Will look at the GPT flags in a moment.
#20 Updated by intrigeri 2019-01-05 11:31:50
- QA Check changed from Ready for QA to Dev Needed
- Nitpicking: to be more consistent with how we write things elsewhere in our test suite, please replace
"([^"]+)"for matching captured bits of step titles.
- API clarity: at least one test takes a
on "$image"argument but does not use it:
/dev/sda1is hard-coded in the test implementation. I think it’s fine to hard-code
/dev/sda1there (assuming we don’t have code yet to find this out dynamically, or we do the same elsewhere already) but then let’s not pretend we’re supporting checking an arbitrary drive, and instead drop that argument.
- Maybe you missed the first part of my review i.e.
#21 Updated by CyrilBrulebois 2019-01-05 15:35:18
#20: points 1 & 2 addressed.
#7: point 1 addressed I think (looping and working on ISO/IMG in the same way, except for
OLD_TAILS_* where factorizing wasn’t really worth it), but 2 still needs addressing. (And yeah, it worked reliably for me until now.)
I’ll get back to that during next time I’m waiting on other
feature/buster things; keeping this Dev Needed for now, but feel free to discover other issues.
#22 Updated by CyrilBrulebois 2019-01-05 21:28:32
- Assignee deleted (
- QA Check changed from Dev Needed to Ready for QA
Just pushed a few more commits:
cbbaf4dad1..eef12f0fb5 HEAD -> test/16003-growing-system-partition
I’ve fixed the guestfs issue for all tests I worked on, introduced a wrapper as discussed elsewhere to organize udisks-provided info into a little tree (I almost stripped the common prefix for the keys for brevity, to get
Block, etc. keys, but finally decided against it). Note the XXX, as we have entries spanning across several lines. I haven’t seen enough examples of
udisksctl info output to conclude it’s safe enough for a default case… We should probably match for leading spaces, strip all leading spaces, and concat that to the previous value with an extra
\n character, and leave the default character as a catch-all which would log/generate an error when encountering an unknown pattern?
Some other tests might benefit from this wrapper, instead of just trying to match the whole output of this command, but that looks like out of scope for this specific task.