Feature #10151

Organize automated test suite artifacts in per-run directories

Added by anonym 2015-09-02 12:52:33 . Updated 2015-11-03 11:30:29 .

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

100%

Feature Branch:
test/10151-organize-artifacts
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

A single test suite generally results in multiple files being created in the TMP dir, e.g.:

  • snapshots and disks
  • failure screenshots
  • network captures
  • videos (with Feature #10148)

Some of these try to encode which test run it belonged to by including a timestamp. This is awkward, especially when developing new tests.

I propose that for the artifacts that reflect some sort of final result/log that will not be potentially reused in subsequent runs (e.g. everything above except “snapshots and disks”) we place them in $TMP/run-$TIMESTAMP-$XXXXXX, where $TIMESTAMP is when the test suite run was started and $XXXXXX is some random chars decided mktemp-style.


Subtasks


Related issues

Related to Tails - Feature #10148: Split test suite capture videos per-scenario Resolved 2015-09-02
Related to Tails - Feature #8682: Reorganize the test suite's temporary directory Rejected 2015-01-12
Blocks Tails - Bug #10342: Don't print test suite information to STDERR Resolved 2015-10-06
Blocks Tails - Feature #10445: Pass --artifacts-base-uri in Jenkins test suite wrapper Resolved 2015-10-28

History

#1 Updated by anonym 2015-09-02 12:54:03

anonym wrote:
> we place them in $TMP/run-$TIMESTAMP-$XXXXXX

I guess it’s conceivable to add an --artifacts-dir option to make this configurable too.

#2 Updated by anonym 2015-09-02 16:47:06

  • Status changed from Confirmed to In Progress

Applied in changeset commit:81caf05b3d83485e0e29d42ac5c622e6cd7d86a8.

#3 Updated by anonym 2015-09-02 16:48:39

  • % Done changed from 0 to 20
  • Feature Branch set to test/10151-organize-artifacts

I got ahead of myself and implemented this before the discussion was settled. :) I won’t be sad if we decide against this and this work was for nothing.

#4 Updated by bertagaz 2015-09-03 00:46:22

anonym wrote:
> I got ahead of myself and implemented this before the discussion was settled. :) I won’t be sad if we decide against this and this work was for nothing.

To me this makes perfectly sense. Same page again, I had the same thought while starting to write the first cheap automated test job. :)

For the sake of the deployment in Jenkins, I think a --artifacts-dir option would be of great help, because between the runs this directory will have to be wiped. Otherwise the filesystem will get quickly filled. Good catch!

#5 Updated by anonym 2015-09-03 01:37:27

bertagaz wrote:
> anonym wrote:
> > I got ahead of myself and implemented this before the discussion was settled. :) I won’t be sad if we decide against this and this work was for nothing.
>
> To me this makes perfectly sense. Same page again, I had the same thought while starting to write the first cheap automated test job. :)

:)

> For the sake of the deployment in Jenkins, I think a --artifacts-dir option would be of great help, because between the runs this directory will have to be wiped. Otherwise the filesystem will get quickly filled. Good catch!

It’s already implemented. :)

#6 Updated by bertagaz 2015-09-03 01:47:15

  • Type of work changed from Discuss to Code

anonym wrote:
> bertagaz wrote:
>
> > For the sake of the deployment in Jenkins, I think a --artifacts-dir option would be of great help, because between the runs this directory will have to be wiped. Otherwise the filesystem will get quickly filled. Good catch!
>
> It’s already implemented. :)

Awesome! \o/

#7 Updated by anonym 2015-09-15 13:00:36

  • Assignee set to anonym
  • Target version set to Tails_1.7

#8 Updated by anonym 2015-09-16 12:13:04

  • related to Feature #10148: Split test suite capture videos per-scenario added

#9 Updated by anonym 2015-09-16 12:14:30

  • QA Check set to Dev Needed

Some ideas from Feature #10148:
> > I wonder if we should perhaps display the full path to the video capture upon failure, much like we’ve been doing forever for screenshots. This would save some time when debugging (in particular when --capture-all is used, but also when the temp directory is filled with videos from previous runs). What do you think? Assuming it’s a good idea: is it trivial enough to be dealt with as part of this ticket, or do you want another one?
>
> Definitely. Have a look at Feature #10151’s feature branch, which already does that, and fixes this output to be more ordered and consistent. I think your remaining concerns could be fixed in that branch too, and be ignore for this ticket. If you agree, let’s move this discussion there.
>
> > I also have doubts wrt. the video files naming:
> >
> > * I’m not sure if capture- prefix adds any value, given we have the filename extension already and we don’t put any other videos in that directory AFAIK.
>
> Ok, this makes sense. Same for the screenshot and .pcap files then.
>
> > * Perhaps make it more similar to the screenshots we save? Those files have very similar purposes, and are named very differently with this branch. Maybe we should simply merge both filename formats, to include scenario.feature.file + some separator + scenario.name (and sanitize the result) in all cases?
>
> Sure. We will get very long filenames, but whatever.
>
> > * Video filename embeds TIME_AT_START, while screenshots filename embeds the failure time. It feels a bit confusing. Renaming the video upon failure would be inconsistent as well (wrt. --capture-all this time). Perhaps start time could be used for screenshots as well? This one I’m less sure, feel free to disregard if you don’t think it’s important.
>
> Again, this is done in the sanest way I can think of in Feature #10151.

#10 Updated by anonym 2015-09-25 03:19:39

  • related to Feature #8682: Reorganize the test suite's temporary directory added

#11 Updated by anonym 2015-09-27 05:41:20

After a bit of discussion, we have settled on the following plan:

We organize all artifacts in a per-run sub-directory like this:

  • $TMPDIR/run-${TIMESTAMP_AT_START}-$XXXXXX/
    • ${SANITIZED_SCENARIO_NAME}-${TIMESTAMP_AT_FAILURE}.mp4
    • ${SANITIZED_SCENARIO_NAME}-${TIMESTAMP_AT_FAILURE}.png
    • ${SANITIZED_SCENARIO_NAME}-${TIMESTAMP_AT_FAILURE}.pcap

Where $XXXXXX is a mktemp-style random string. The ${TIMESTAMP_*} ones should be self-explained (but they too must be sanitized vs. unix-hostile chars).

In the log(s) we print upon failure the paths/urls to each artifact in the After (scenario) hook. E.g.:

<cucumber stuff, stack trace>
Scenario failed at time ${TIMESTAMP_AT_FAILURE}
Screenshot: $TMPDIR/run-${TIMESTAMP_AT_START}-$XXXXXX/${SANITIZED_SCENARIO_NAME}-${TIMESTAMP_AT_FAILURE}.png
Video:      $TMPDIR/run-${TIMESTAMP_AT_START}-$XXXXXX/${SANITIZED_SCENARIO_NAME}-${TIMESTAMP_AT_FAILURE}.mp4
pcap file:  $TMPDIR/run-${TIMESTAMP_AT_START}-$XXXXXX/${SANITIZED_SCENARIO_NAME}-${TIMESTAMP_AT_FAILURE}.pcap

We add these options to run_test_suite:

  • --artifacts-dir DIR: Store the artifacs in DIR instead of $TMPDIR/run-${TIMESTAMP_AT_START}-$XXXXXX/. (I will probably skip this option to reduce complexity since it won’t be very helpful for the jenkins deployment, at least how I understand it. Please explicitly request it from me if you are interested in having this option.)
  • --artifacts-base-url URL: when printing paths to the errors, instead we print urls with the $TMPDIR/run-${TIMESTAMP_AT_START}-$XXXXXX/ replaced with URL. This will be useful in our jenkins setup, to point to somewhere these artifacts can be fetched. Example with --artifacts-baseurl https://jenkins.tails.boum.org/job/test_Tails_ISO_isotester4/41/artifact/build-artifacts:
Screenshot: https://jenkins.tails.boum.org/job/test_Tails_ISO_isotester4/41/artifact/build-artifacts/${SANITIZED_SCENARIO_NAME}-${TIMESTAMP_AT_FAILURE}.png
...

#12 Updated by anonym 2015-10-01 16:31:15

Since I’ll also fix how we print these things, we could use this function:

require 'cucumber/formatter/console'
def puts_info(m = "")
  # This trick allows us to use a module's (~private) method on a one-off basis.
  puts Class.new.extend(Cucumber::Formatter::Console).format_string(m, :white)
end


Some formatters (like pretty and our debug) will make use of things written to STDOUT and write it at an appropriate place, unlike STDERR which will only be printed to the terminal => this info doesn’t end up in files when we log to them. In fact, while we’re at it, we can eliminate all instances where we print to STDERR.

#13 Updated by anonym 2015-10-06 06:09:18

anonym wrote:
> Since I’ll also fix how we print these things, we could use this function:
> […]
> Some formatters (like pretty and our debug) will make use of things written to STDOUT and write it at an appropriate place, unlike STDERR which will only be printed to the terminal => this info doesn’t end up in files when we log to them. In fact, while we’re at it, we can eliminate all instances where we print to STDERR.

Created Bug #10342 for this.

#14 Updated by anonym 2015-10-06 08:45:43

Applied in changeset commit:7dd7582dddd1ae62a0c4630395ca5891a61c9100.

#15 Updated by anonym 2015-10-06 08:45:44

Applied in changeset commit:5a2e101840fa2de8214d3b6d811608163e544f16.

#16 Updated by anonym 2015-10-06 08:47:12

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

#17 Updated by anonym 2015-10-06 09:23:09

Hmm, it seems I dropped the timetamp from the artifact files, so we get e.g. ${SANITIZED_SCENARIO_NAME}.png instead of ${SANITIZED_SCENARIO_NAME}-${TIMESTAMP_AT_FAILURE}.png. I think that’s good. But it would be problematic if we had scenarios with identical names, but we really should not do that. What do you think?

#18 Updated by anonym 2015-10-08 09:32:24

anonym wrote:
> Hmm, it seems I dropped the timetamp from the artifact files, so we get e.g. ${SANITIZED_SCENARIO_NAME}.png instead of ${SANITIZED_SCENARIO_NAME}-${TIMESTAMP_AT_FAILURE}.png. I think that’s good. But it would be problematic if we had scenarios with identical names, but we really should not do that. What do you think?

What about ${TIMESTAMP_AT_FAILURE}-${SANITIZED_SCENARIO_NAME}.png? Then lexical sort == chronological sort, and the artifacts will group per scenario in order of failure time with a simple ls, which seems pretty nice to me.

#19 Updated by anonym 2015-10-08 09:37:36

Also, I think I’d want the commit that the test suite was started from in the artifacts folder name, e.g.: $TMPDIR/run-${TIMESTAMP_AT_START}-git_${GIT_SHORTID}-${XXXXXX}. I sometimes “queue” multiple runs, and this would help me. Example:

/tmp/TailsToaster/run-2015-10-08_10:41:16_+0200-git_acfe3f5-NbsVD1

Hmm. And the branch/tag name? $TMPDIR/run-${TIMESTAMP_AT_START}-git_${GIT_BRANCH_OR_TAG}_${GIT_SHORTID}-${XXXXXX}. Example:

/tmp/TailsToaster/run-2015-10-08_10:41:16_+0200-git_experimental_acfe3f5-NbsVD1

#20 Updated by bertagaz 2015-10-11 02:39:22

  • blocks Bug #10356: Adapt the test job artifacts archiving script to their new location added

#21 Updated by anonym 2015-10-13 11:47:55

I ended up implementing both

> What about ${TIMESTAMP_AT_FAILURE}-${SANITIZED_SCENARIO_NAME}.png? Then lexical sort == chronological sort, and the artifacts will group per scenario in order of failure time with a simple ls, which seems pretty nice to me.

and

> And the branch/tag name? $TMPDIR/run-${TIMESTAMP_AT_START}-git_${GIT_BRANCH_OR_TAG}_${GIT_SHORTID}-${XXXXXX}.

Here’s an actual example from a test run:

      Scenario failed at time 00:05:22

      Network capture: /home/tailstester-rjb/TailsToaster/run-2015-10-13_20:12:26_+0200-git_test-10151-organize-artifacts_b34cfdb-CL4KvD/00:05:22_Anti_test:_Detecting_IPv4_TCP_leaks_from_the_Unsafe_Browser_with_the_firewall_leak_detector.pcap

      Screenshot: /home/tailstester-rjb/TailsToaster/run-2015-10-13_20:12:26_+0200-git_test-10151-organize-artifacts_b34cfdb-CL4KvD/00:05:22_Anti_test:_Detecting_IPv4_TCP_leaks_from_the_Unsafe_Browser_with_the_firewall_leak_detector.png

      Video: /home/tailstester-rjb/TailsToaster/run-2015-10-13_20:12:26_+0200-git_test-10151-organize-artifacts_b34cfdb-CL4KvD/00:05:22_Anti_test:_Detecting_IPv4_TCP_leaks_from_the_Unsafe_Browser_with_the_firewall_leak_detector.mkv

Yes, it is indeed pretty verbose, but given the amount of test suite runs I do, it will help immensely in keeping track of what I actually was testing for a particular run. I wouldn’t be surprised if kytv also would appreciate this. :)

#22 Updated by intrigeri 2015-10-13 13:32:00

> Yes, it is indeed pretty verbose, but given the amount of test suite runs I do, it
> will help immensely in keeping track of what I actually was testing for a particular
> run. I wouldn’t be surprised if kytv also would appreciate this. :)

That’s a pretty convincing reason :)

#23 Updated by bertagaz 2015-10-14 02:39:44

  • blocked by deleted (Bug #10356: Adapt the test job artifacts archiving script to their new location)

#24 Updated by bertagaz 2015-10-14 05:47:18

It seems that commit 8312aba brings troubles in the test suite in Jenkins (see test job #19 of the experimental branch).

I suspect the new way to detect the branch doesn’t like when we reset the branch to a specified commit, and then merge a specified commit of its base branch (devel) in it, which is the way in Jenkins we ensure to test ISOs using the same git repo state they were build from.

#25 Updated by intrigeri 2015-10-14 05:56:22

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

> It seems that commit 8312aba brings troubles in the test suite in Jenkins […]

Putting back onto anonym’s plate, then.

#26 Updated by anonym 2015-10-14 06:19:16

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

bertagaz wrote:
> It seems that commit 8312aba brings troubles in the test suite in Jenkins (see test job #19 of the experimental branch).
>
> I suspect the new way to detect the branch doesn’t like when we reset the branch to a specified commit, and then merge a specified commit of its base branch (devel) in it, which is the way in Jenkins we ensure to test ISOs using the same git repo state they were build from.

Fixed. We simply omit the branch/tag part if in detached HEAD. Also merged into experimental.

#27 Updated by bertagaz 2015-10-14 13:41:54

anonym wrote:
> Fixed. We simply omit the branch/tag part if in detached HEAD. Also merged into experimental.

Since then the tests are running better for this branch, so I guess case closed on the Jenkins side :)

#28 Updated by anonym 2015-10-15 07:20:12

  • related to Bug #10342: Don't print test suite information to STDERR added

#29 Updated by kytv 2015-10-26 08:19:13

  • Assignee changed from kytv to intrigeri

Code review and testing passed.

#30 Updated by intrigeri 2015-10-27 14:55:30

  • related to deleted (Bug #10342: Don't print test suite information to STDERR)

#31 Updated by intrigeri 2015-10-27 14:55:39

  • blocks Bug #10342: Don't print test suite information to STDERR added

#32 Updated by intrigeri 2015-10-27 15:27:20

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

(bertagaz: there’s something for you below, look for your name.)

I did a code review (not checking every line that was moved around, because I count on kytv to have done that) and have run a couple tests.

First, I like the new layout and filenames very much, yay!

I notice that my suggestion to include the feature title in the filename has been dropped, and JFTR I’m OK with what was chosen instead (worst case, this will encourage us to improve on our scenario naming skills ;)

I see:

+def info_log(message = "", options = {})
+  options[:color] = :white

What’s the intended result on a terminal with a white background? On mine, I see very light gray on a white background.

I think my first test triggered a case when no info about the artifacts is displayed on failure:

# ./run_test_suite --capture --pause-on-fail --view --iso ../tails-i386-devel-1.7-20151012.iso features/apt.feature:20 -- --format pretty --format debug --out debug.log
Virtual X framebuffer started on display :0
VNC server running on: localhost:5900
[info] Sikuli vision engine loaded.
@product
Feature: Installing packages through APT
  As a Tails user
  when I set an administration password in Tails Greeter
  I should be able to install packages using APT and Synaptic
  and all Internet traffic should flow only through Tor.

  Background:                                                                                                      # features/apt.feature:8
    Given I have started Tails from DVD and logged in with an administration password and the network is connected # features/step_definitions/snapshots.rb:174

  @check_tor_leaks
  Scenario: Install packages using Synaptic                                                                        # features/apt.feature:20
    When I start Synaptic                                                                                          # features/step_definitions/apt.rb:61
    And I update APT using Synaptic                                                                                # features/step_definitions/apt.rb:31
      FindFailed: can not find SynapticPackageList.png on the screen.
      Line ?, in File ? (RuntimeError)
      features/apt.feature:22:in `And I update APT using Synaptic'
    Then I should be able to install a package using Synaptic                                                      # features/step_definitions/apt.rb:45

Press ENTER to continue running the test suite


Failing Scenarios:
cucumber features/apt.feature:20 # Scenario: Install packages using Synaptic

1 scenario (1 failed)
4 steps (1 failed, 1 skipped, 2 passed)
9m25.893s

I confirm I have a video + screenshot in /tmp/TailsToaster/run-2015-10-27_17\:42\:24_-0400-git_testing_4682e2b-bJB94i/.

Then I’ve run another failing scenario, without --pause-on-fail nor --capture this time, and this time I did see “Scenario failed at time” and the path to a screenshot. Could it be that this branch is not compatible with --pause-on-fail, or similar?

Typo: git grep artifacs.

Given how descriptive the filenames are, I guess that sometimes I’d like to just cd into the artifacts directory and have a look at some artifacts, before I even bother reading the corresponding log. So I was a bit surprised not to be told, at the end of a run, what’s the artifacts base directory: it implies that I have to scroll up to the last failure to find this information. If you think this would be worthwhile and find it easy to implement, please do. Otherwise, forget it.

I would s/artifacts-base-url/artifacts-base-uri/ but well, you know :)

To end with, I see no ticket blocked by this one, about using --artifacts-base-url in our Jenkins deployment. bertagaz, please make sure you have a ticket about it.

#33 Updated by anonym 2015-10-28 05:30:38

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

intrigeri wrote:
> I notice that my suggestion to include the feature title in the filename has been dropped, and JFTR I’m OK with what was chosen instead (worst case, this will encourage us to improve on our scenario naming skills ;)

Sorry, I forgot about that. :S

> What’s the intended result on a terminal with a white background? On mine, I see very light gray on a white background.

To force them to switch to a sane, black/grey terminal background? :P Fixed in commit:1fe6b72.

Next up: supporting green and red terminal backgrounds… kidding! :)

> I think my first test triggered a case when no info about the artifacts is displayed on failure:
>
> […]
>
> I confirm I have a video + screenshot in /tmp/TailsToaster/run-2015-10-27_17\:42\:24_-0400-git_testing_4682e2b-bJB94i/.

I cannot reproduce this, using the same command line as you (and modifying the step so it always fails via a raise). Can you please try to reproduce?

> Then I’ve run another failing scenario, without --pause-on-fail nor --capture this time, and this time I did see “Scenario failed at time” and the path to a screenshot. Could it be that this branch is not compatible with --pause-on-fail, or similar?
>
> Typo: git grep artifacs.

commit:783c54b

> Given how descriptive the filenames are, I guess that sometimes I’d like to just cd into the artifacts directory and have a look at some artifacts, before I even bother reading the corresponding log. So I was a bit surprised not to be told, at the end of a run, what’s the artifacts base directory: it implies that I have to scroll up to the last failure to find this information. If you think this would be worthwhile and find it easy to implement, please do. Otherwise, forget it.

To have it printed at the very end, after the scenario/step summary, is a trivial one-line thing. However, I want it printed just before that summary, which is a bit more involved, but I did it in commit:e857718. Not very elegant, but oh well.

> I would s/artifacts-base-url/artifacts-base-uri/ but well, you know :)

commit:7c6c892

> To end with, I see no ticket blocked by this one, about using --artifacts-base-url in our Jenkins deployment. bertagaz, please make sure you have a ticket about it.

bert?

#34 Updated by bertagaz 2015-10-28 10:16:28

  • blocks Feature #10445: Pass --artifacts-base-uri in Jenkins test suite wrapper added

#35 Updated by bertagaz 2015-10-28 10:20:00

anonym wrote:
> intrigeri wrote:
> > To end with, I see no ticket blocked by this one, about using --artifacts-base-url in our Jenkins deployment. bertagaz, please make sure you have a ticket about it.
>
> bert?

Done. Forgot this option made its way there.

#36 Updated by intrigeri 2015-10-31 06:49:15

  • Assignee changed from intrigeri to anonym
  • QA Check changed from Ready for QA to Info Needed

Code review OK up to commit:652abb09ed171040c01b9ba29c1e47dca807c8d3.

>> I think my first test triggered a case when no info about the artifacts is displayed on failure:
>>
>> […]
>>
>> I confirm I have a video + screenshot in /tmp/TailsToaster/run-2015-10-27_17\:42\:24_-0400-git_testing_4682e2b-bJB94i/.

> I cannot reproduce this, using the same command line as you (and modifying the step so it always fails via a raise). Can you please try to reproduce?

I reproduced it. I didn’t have to add a raise, and I wonder if this matters. Note that when asked to “Press ENTER to continue running the test suite” I did press ENTER, and this time:

Press ENTER to continue running the test suite


Artifacts directory: /tmp/TailsToaster/run-2015-10-31_08:46:12_-0400-git_testing_652abb0-gJtenb

Failing Scenarios:
cucumber features/apt.feature:20 # Scenario: Install packages using Synaptic

1 scenario (1 failed)
4 steps (1 failed, 1 skipped, 2 passed)
19m37.772s

I wanted to reproduce on another system, but Bug #10460 prevented me to.

Does the exact ISO I’m testing matter?

#37 Updated by anonym 2015-10-31 13:04:52

  • Assignee changed from anonym to intrigeri
  • QA Check changed from Info Needed to Ready for QA

intrigeri wrote:
> I reproduced it. […]

Despite running Debian Jessie, I have been running Sid’s cucumber for a while (since I experimented Bug #10281) so this branch has been completely developed for cucumber ~2.0.0. Indeed, in <2.0.0 there’s a bug that we hit, so stuff written via puts is either delayed to the next Scenario/Feature, or dropped completely, like in this case where it happened in the last Scenario of the last feature. I’ve now downgraded to Jessie’s cucumber, and fixed the issue in commit:be6aecd.

#38 Updated by intrigeri 2015-11-01 01:25:10

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

Applied in changeset commit:cf79bcb8080503f188fa8f82ee9a795396a4fdc2.

#39 Updated by intrigeri 2015-11-01 01:26:27

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

Wow, good catch! (We’re quite lucky that I happened to hit this bug actually.)

#40 Updated by anonym 2015-11-03 11:30:29

  • Status changed from Fix committed to Resolved