Feature #8188

Improve test suite configuration

Added by anonym 2014-10-30 17:44:29 . Updated 2015-02-24 22:47:59 .

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

100%

Feature Branch:
test/8188-better-test-suite-configuration
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

Currently test suite configuration (like whether debugging is enabled, and the path to the image to test) are stored among several global variables, and how they get their value is quite ad-hoc.

In the test/better-configuration branch I’ve made the situation a bit better and made it more well-defined and clearer (including documentation) how environment variables can be set to configure the test suite.

It also adds adds a local configuration file were these values can be set. The precedence between the different ways to set the variables (the file, shell environment, run_test_suite parameters) is now more well-defined and also documented.

The local configuration file is also the intended way to store secrets to be used in tests, currently only bridge lines. In the future we may end up storing secret credentials for web services we want to test without making those secrets public. For instance, to test git we may store something like:

GIT_SSH_URL=tails@git.tails.boum.org:test-repo
GIT_SSH_PUBLIC_KEY=ssh-rsa [...]
GIT_SSH_PRIVATE_KEY_SERIALIZED=-----BEGIN RSA PRIVATE KEY-----\n[...]

so we can automate the Git+SSH test using a real, live Git repo (as opposed what’s suggested in Feature #6300). We could have a canonical local configuration file to be used when testing new test releases.


Subtasks


Related issues

Related to Tails - Feature #6301: Securely store secrets needed by the automated test suite Resolved 2013-09-26

History

#1 Updated by intrigeri 2014-10-31 12:59:23

> The local configuration file is also the intended way to store secrets to be used in tests, currently only bridge lines. […]

Great!

>

> GIT_SSH_URL=tails@git.tails.boum.org:test-repo
> GIT_SSH_PUBLIC_KEY=ssh-rsa [...]
> GIT_SSH_PRIVATE_KEY_SERIALIZED=-----BEGIN RSA PRIVATE KEY-----\n[...]
> 

I suggest we use a more structured configuration file format, such as YAML.

#2 Updated by anonym 2014-11-06 12:34:38

intrigeri wrote:
> I suggest we use a more structured configuration file format, such as YAML.

Some of the simplicity of this solution is that the format of the local configuration file is the same as the shell environment. I thought using a more sophisticated format for the configuration file would complicate things since we’d have to use two different formats.

However, if we just decide that the root of the YAML file is an associative array (which is how the shell environment works), and that the shell environment uses YAML-syntax for the values, then we should have something that works. In fact, this may make things even simpler since we can piggyback on YAML’s type autodetection and not have to think of it.

So, the branch now uses YAML for this file.

#3 Updated by intrigeri 2014-11-06 12:36:21

> So, the branch now uses YAML for this file.

\o/

#4 Updated by anonym 2015-01-09 14:29:26

  • related to Feature #6301: Securely store secrets needed by the automated test suite added

#5 Updated by intrigeri 2015-02-12 14:42:18

Implementation note: if the branch for Feature #8670 hard-codes SSH keys in step definition and is merged before this one, then the branch for this very ticket should move it away to some saner place.

#6 Updated by anonym 2015-02-18 16:16:09

  • % Done changed from 30 to 50
  • Feature Branch changed from test/better-configuration to test/8188-better-test-suite-configuration

I’ve completely re-worked this in a new branch.

#7 Updated by anonym 2015-02-19 09:03:53

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

I’m asking intrigeri for review of this branch, cause we had some private discussions on how it should be designed. kytv, please have a look any way.

#8 Updated by intrigeri 2015-02-19 12:35:38

  • Assignee changed from intrigeri to anonym
  • % Done changed from 50 to 70
  • QA Check changed from Ready for QA to Dev Needed

Thanks for the atomic commits!

Code review passes, except the usage documentation (wiki/src/contribute/release_process/test/usage.mdwn) wasn’t updated; IMO it should at least document:

  • the local config file location and syntax;
  • that the local config file can override defaults, and in turn it can be overriden by environment variables.

I’ve also done a full run of the automated test suite, that highlighted some issues that anonym fixed since then, so I’m currently running the failing tests again.

#9 Updated by intrigeri 2015-02-19 13:21:07

Only usb_install.feature left to run.

#10 Updated by anonym 2015-02-19 14:01:10

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

intrigeri wrote:
> Thanks for the atomic commits!

:)

> Code review passes, except the usage documentation (wiki/src/contribute/release_process/test/usage.mdwn) wasn’t updated; IMO it should at least document:
>
> * the local config file location and syntax;
> * that the local config file can override defaults, and in turn it can be overriden by environment variables.

Ah, sorry. That was the last part from the old branch (test/better-configuration) that I was gonna adapt for the new one, but forgot. Now pushed.

Note that we don’t support configuration through environment variables. I dropped that because it would just make things complicated, for some doubtful gain.

#11 Updated by intrigeri 2015-02-19 14:21:45

  • Assignee changed from intrigeri to anonym
  • % Done changed from 70 to 80
  • QA Check changed from Ready for QA to Dev Needed

anonym wrote:
> Now pushed.

  • typo in “non-secret key-value paris”
  • the doc about secrets is good, but why not provide at least one example? because we don’t have any yet at this stage?
  • “the test suite run is suspended” … += “on failure”?
  • “or when to extract new images” feels weird. “when extracting”?

#12 Updated by intrigeri 2015-02-19 15:02:58

  • % Done changed from 80 to 90

intrigeri wrote:
> Only usb_install.feature left to run.

Passed, so I’m going to merge this branch as soon as the doc is fixed.

#13 Updated by anonym 2015-02-19 15:09:43

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

intrigeri wrote:
> anonym wrote:
> > Now pushed.
>
> * typo in “non-secret key-value paris”

Fixed.

> * the doc about secrets is good, but why not provide at least one example? because we don’t have any yet at this stage?

Yeah, we don’t have any yet. I’ll add the first with the bridge branch, I suspect.

> * “the test suite run is suspended” … += “on failure”?
> * “or when to extract new images” feels weird. “when extracting”?

Both fixed.

#14 Updated by Tails 2015-02-19 15:24:11

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

Applied in changeset commit:76ecac141a98aeb343bcdf7f10b2fac52e3c6a89.

#15 Updated by intrigeri 2015-02-19 15:32:30

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

#16 Updated by BitingBird 2015-02-24 22:47:59

  • Status changed from Fix committed to Resolved