[Tails-dev] [Was: [review'n'merge:1.1] #6559, #7062, #6275: …

Delete this message

Reply to this message
Author: intrigeri
Date:  
To: The Tails public development discussion list
Subject: [Tails-dev] [Was: [review'n'merge:1.1] #6559, #7062, #6275: test/6559-adapt-test-suite-for-Wheezy]
Hi,

anonym wrote (13 May 2014 15:54:22 GMT) :
> OT: could you just clarify; why are we gonna stop using kytv's repo?


When we had a lenghty discussion about the I2P packages
importing/sourcing/whatever process on tails-dev@ a few months ago,
one of my (implicit?) assumptions was that we have not (yet) built
enough trust into kytv to allow them to drop file anywhere on the
Tails filesystem.

I'm sure they won't take offense of seeing me reiterating this, even
if it feels potentially hard to hear.

> At least that's what I interpret the ticket as, that we're gonna go
> back to manually importing the packages.


Based on the above, yes. That's why I have documented, IIRC, how to
check these packages to verify that they don't put files in any
unexpected place, and therefore should not affect (if I got it right)
users who don't opt-in to start I2P.

> Is it for providing sources?


We actually import source and binary packages from kytv's APT repo.

>> Regarding the removal of test suite steps that look at notifications:
>>
>>   1. It seems to work on my hardware + current Internet connection (up
>>      to the features I've already tested), and it certainly works on
>>      yours. But IIRC these steps were added to avoid the notifications
>>      breaking things, when they appear concurrently with another app's
>>      window that we want to find on the screen. I'm not sure if we can
>>      confident that this won't happen anymore on slower hardware, or
>>      Internet connection. I don't object to merging this as is, but
>>      I'm a bit afraid this bites us in the future. We'll see.


> Unfortunately the hardware argument goes both ways -- on slow hardware
> Sikuli might not find the notifications before they have disappeared
> thanks to GNOME3's greatly reduced timeout (no matter what's set
> manually as the timeout).


> The other problem is when we try to close them -- they may disappear
> right before the click, and then the click may shift application focus,
> or click on something else (perhaps the X-button of a window, which
> often is in the notification areas vicinity). BTW, this could only
> happen in "I see and close the Unsafe Browser start notification" of the
> removed tests. Luckily the reduced timeout is our friend here since we
> then can just waitVanish instead of clicking.


Right, (very good) points taken. Once again, you demonstrate that you
have a waaaay deeper understanding of our test suite than me :)

/me crosses fingers, and hopes we can convert all these stateful
things to systemd units at some point, and then have a way to ask the
system under testing, from the outside, if this or that has been
completed already. I guess that for the notifications that are run
from some /etc-ish thing, it should be possible on Wheezy, but for
those that are run from the user session, we'll have to wait for
systemd to manage the user session too, which should be available (as
opt-in, IIRC) on Jessie. That could be yet another good reason to
migrate to Jessie faster than we did with Wheezy.

>>   2. These steps were also useful to... actually test that we're
>>      displaying these notifications. In the context of the test suite,
>>      they are annoying, but for actual users, they are features.
>>      We have dropped tests such as "I see a notification that Unsafe
>>      browser is starting" from our manual test suite when the
>>      automated test suite implemented it. And now, we're dropping this
>>      test from the automated test suite too. I've not carefully
>>      checked if other similar tests were removed. That looks like
>>      a regression to me. What do you think?


> ACK. I'll bring it back tomorrow with some needed modifications.


Thank you. I won't be able to test and review this before May 24, so
better nag bertagaz to take care of it (at least of the testing part,
if he doesn't feel comfortable reviewing the thing).

>> It's not clear to me what commit 6cecfe6 ("Do not purge bind9-host's
>> dependencies") has to do in this branch. The commit itself makes
>> sense, so I don't mind, but I felt like raising it to make sure it
>> wasn't a mistake.


> Sorry for being unclear. Without that commit we're left with no
> /usr/bin/host in Wheezy, which firewall_checks.feature depends on. This
> is why I said the images to be tested must be built from this branch.


Makes sense.

>> I'm not convinced by your rewriting (17f2a98) of the commit message
>> I wrote in 1c5991c, losing useful info along the way, but oh well, who
>> cares. It feels sub-optimal that the erroneous commit message for
>> cdbee937 was not fixed, while you were at rewriting history. Same for
>> 4c7beba + its revert commit, that could have been dropped. No big
>> deal, probably. </nitpicking> (or not, I've more!)


> Note that I didn't rewrite *my* branch's history, in case you had
> already started looking at it. Rewriting it now may be too late, unless
> you dare relying on a `git diff` between your local branch and a force
> pushed one (if the diff is empty up until the commit that corresponds to
> the HEAD of your local branch, all should be ok). Your call.


No, let's not rewrite things now.

>> I think that the error message raised in /^process "([^"]+)" is
>> running within (\d+) seconds$/ should be different from the one in
>> /^process "([^"]+)" is running$/, and it should convey the "N seconds"
>> information. What do you think?


> I agree, now fixed.


>> The commit message ("Clean up.") for 4cd0017f does not tell me why
>> short command-line options disappear.


> As you can see the short options do nothing and never did. My only
> explanation is that I committed them by mistake when I initially
> getopt:ified the script (commit e273f40).


Thanks for the explanation. Please make this clear in the commit
message next time, which will save both of us some time :)

>> I'm not 100% convinced by the way @skip works. [...]


> Done. See new thread "How to deal with tests expected to fail?" for more
> on this. I'll open a ticket as soon as it hits the archive so I can link
> to my conclusions. IMHO this can wait until after 1.1 since we have more
> pressing matters until then (at least).


Definitely. This would perhaps be a good candidate for 2.0, as a way
to improve maintainability, and to allow us to run our test suite on
a CI server. Anyway, we'll discuss this at the summit!

>> I don't understand the complexity added by commit bc6805ec, and the
>> commit message tells me nothing about it. Have you seen actual cases
>> when TailsData was *not* unlocked+mounted, after t-p-s exited?


> That step is also used in scenarios when t-p-s isn't run at all, and
> persistence is disabled, see e.g. "Booting Tails from a USB drive with a
> disabled persistent partition".


OK... but I still don't get why some action is needed on Wheezy, or
rather, I don't get why no action was needed on Squeeze, given what
I've reported on #7056.

Cheers!
--
intrigeri
| GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
| OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc