(FTR that's #5465.)
intrigeri wrote (27 Jul 2013 16:59:39 GMT) :
> anonym wrote (26 Jul 2013 12:47:46 GMT) :
>> branch: test/fix-persistence-checks
> I've had a look. This changeset generally looks generally good to me
> (though I haven't tested it yet, mostly because I don't expect my test
> setup to be able to go as far as testing these steps...).
> Nitpicking follows.
>> + return @vm.execute("test -e #{persistence_state_file}").success? &&
>> + @vm.execute(". #{persistence_state_file}; " +
>> + "test $TAILS_PERSISTENCE_ENABLED = true").success?
> Due to the lack of quotes around $TAILS_PERSISTENCE_ENABLED, if the
> tested condition is not verified (e.g. persistence_state_file is
> missing empty or something), then I guess that the *shell* will error
> out due to a syntax error, which IMHO is poor reporting from a test
> suite: in error conditions, I think a test suite should tell us that
> what we're testing is not verified, e.g. something like
> "$TAILS_PERSISTENCE_ENABLED is not set to true as it is supposed to
> be", instead of just erroring out with "parse error: condition
> expected: =" because it could actually *not* perform a test.
> What do you think?
> In the same spirit, perhaps '&&' could be used instead of ';' to
> chain commands in there.
Done in that branch.
> To end with, I fail to see the rationale to split the "all persistent
> directories are mounted" step out of "persistence is enabled". I think
> it's the kind of low-level implementation detail that makes it harder
> to read and maintain features, and that should instead be hidden in
> step definitions. It's probably not in these terms that a Tails user
> would express their experience of the Tails persistence feature.
> What do you think?
I think I'll just revert this part unless someone objects.
> I'm marking #5465 as "dev needed", and reassigning to anonym.
I'll bring this branch to completion.
Cheers,
--
intrigeri
| GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
| OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc
