[Fuego] [PATCH 0/8] fuego-release-test fixes

[Fuego] [PATCH 0/8] fuego-release-test fixes

[Guilherme Campos Camargo]

Hello, Tim

These patches are implementing the changes you've requested in your
review and a few other fixes, in summary:

  - Replace working_dir with two separate dirs: resources_dir (for test
    assets) and output_dir (for test resulting files)

  - Print the results of the tests individually, prefixing them with an
    id and with a summary - and parse them individually with log_compare
    ("ok" and "not ok")

    Output Example (Log level set as INFO)

    ```
    ...
    test_run:REPORT: run 15 check text build queue fuego board check
    test_run:INFO: Executing Selenium Command 'CheckText'
    test_run:INFO:   Matching text found
    test_run:REPORT: ok 15
    test_run:REPORT: run 16 click img build hello world
    test_run:INFO: Executing Selenium Command 'Click'
    test_run:INFO:   Element clicked
    test_run:REPORT: ok 16
    test_run:REPORT: run 17 check text executors hello world
    test_run:INFO: Executing Selenium Command 'CheckText'
    test_run:INFO:   Matching text found
    test_run:REPORT: ok 17
    test_run:REPORT: run 18 check screenshot side-panel-tasks
    test_run:INFO: Executing Selenium Command 'CheckScreenshot'
    test_run:INFO:   Resulting difference (0.00123834) below threshold (0.1)
    test_run:INFO:   Element's screenshot matches the reference
    test_run:REPORT: ok 18
    test_run:REPORT: run 19 check screenshot footer
    test_run:INFO: Executing Selenium Command 'CheckScreenshot'
    test_run:INFO:   Resulting difference (0.0) below threshold (0.1)
    test_run:INFO:   Element's screenshot matches the reference
    test_run:REPORT: ok 19
    test_run:INFO: All tests finished with SUCCESS
    ```

    Output Example (Log level set as REPORT)

    ```
    ...
    test_run:REPORT: run 15 check text build queue fuego board check
    test_run:REPORT: ok 15
    test_run:REPORT: run 16 click img build hello world
    test_run:REPORT: ok 16
    test_run:REPORT: run 17 check text executors hello world
    test_run:REPORT: ok 17
    test_run:REPORT: run 18 check screenshot side-panel-tasks
    test_run:REPORT: ok 18
    test_run:REPORT: run 19 check screenshot footer
    test_run:REPORT: ok 19

  - Do not require running the test with sudo - See the commit message
    `Remove requirement of running the test script as sudo`

  - Implement a few other minor fixes as updating the readme and catch
    an PermissionError on image.save().

Please let me know what you think.

Thanks

--
Guilherme

Guilherme Campos Camargo (8):
  Update ip example of README.md
  Replace working_dir arg with resources_dir and output_dir
  Remove requirement of running the test script as sudo
  Catch PermissionError when saving screenshots
  Add new log level REPORT
  Add `description` optional argument to test cases
  Add description to tests in COMMANDS_TO_TEST
  Log test results with the REPORT log level and print descriptions

 .../Functional.fuego_release_test/README.md   |  45 ++---
 .../fuego_test.sh                             |  13 +-
 .../Functional.fuego_release_test/test_run.py | 159 +++++++++++++-----
 3 files changed, 144 insertions(+), 73 deletions(-)

-- 
2.17.0

[Fuego] [PATCH 1/8] Update ip example of README.md

[Guilherme Campos Camargo]

Update the ip output example on README.md to match the current
implementation.

Signed-off-by: Guilherme Campos Camargo <guicc@profusion.mobi>
---
 engine/tests/Functional.fuego_release_test/README.md | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/engine/tests/Functional.fuego_release_test/README.md b/engine/tests/Functional.fuego_release_test/README.md
index 1ea6ab3..01942ba 100644
--- a/engine/tests/Functional.fuego_release_test/README.md
+++ b/engine/tests/Functional.fuego_release_test/README.md
@@ -63,14 +63,12 @@ $ ./test_run.py --no-rm-container -p 8080 -w . ~/fuego
 Output example with the IP of the container:
 ```
 ...
-test_run:DEBUG: Container 'fuego-release-container' created
-test_run:DEBUG: Starting container 'fuego-release-container'
-test_run:DEBUG: Running fetch_ip()
-test_run:DEBUG:   Try number 1...
-test_run:DEBUG:   Success
-test_run:DEBUG: Trying to reach jenkins at container 'fuego-release-container' via the container's IP '172.17.0.2' at port '8080'
-test_run:DEBUG: Running ping_jenkins()
+test_run:INFO: Container 'fuego-release-container' successfully created
+test_run:INFO: Starting container 'fuego-release-container'...
+test_run:INFO: Container started with the ip '172.17.0.2'
+test_run:INFO: Waiting for jenkins on '172.17.0.2:8080'...
 ...
+
 ```
 
 Docker commands will be executed from within the script. For that reason, you
-- 
2.17.0

[Fuego] [PATCH 2/8] Replace working_dir arg with resources_dir and output_dir

[Guilherme Campos Camargo]

The generic working_dir argument has been replaced with two separate
arguments: resources_dir and output_dir, the first being the root for
the assets used by the test cases and the second being the location
where test temporary files will be stored (as diff and test images from
CheckScreenshot).

Signed-off-by: Guilherme Campos Camargo <guicc@profusion.mobi>
---
 .../Functional.fuego_release_test/README.md   | 23 ++++----
 .../fuego_test.sh                             |  5 +-
 .../Functional.fuego_release_test/test_run.py | 57 ++++++++++++-------
 3 files changed, 49 insertions(+), 36 deletions(-)

diff --git a/engine/tests/Functional.fuego_release_test/README.md b/engine/tests/Functional.fuego_release_test/README.md
index 01942ba..d9cb7b6 100644
--- a/engine/tests/Functional.fuego_release_test/README.md
+++ b/engine/tests/Functional.fuego_release_test/README.md
@@ -45,19 +45,20 @@ installation directory.
 See examples below:
 
 Test a `Fuego` version that's located at `~/fuego` and use the current
-directory as working-dir (location where test assets will be saved and looked
-for (screenshots, for example).
+directory as resources-dir - location where test assets will be looked for
+(screenshots, for example) - and as output dir (where temporary files resulted
+from the test will be stored).
 ```
-$ ./test_run.py -w . ~/fuego
+$ ./test_run.py -d . -o . ~/fuego
 ```
 
 Same as above, but do not remove (neither stops) the `fuego-container` after
-the execution of the tests. You'll be able to access the inner Jenkins (Fuego
-that runs inside Fuego) through `${fuego_container_ip}:8080/fuego`. The
-`fuego_container ip` is dynamically set by docker, and is displayed in the
-first lines of the output of the script.
+the execution of the tests, and use `/tmp` as output dir (default). You'll be
+able to access the inner Jenkins (Fuego that runs inside Fuego) through
+`${fuego_container_ip}:8080/fuego`. The `fuego_container ip` is dynamically set
+by docker, and is displayed in the first lines of the output of the script.
 ```
-$ ./test_run.py --no-rm-container -p 8080 -w . ~/fuego
+$ ./test_run.py --no-rm-container -p 8080 -d . ~/fuego
 ```
 
 Output example with the IP of the container:
@@ -133,9 +134,11 @@ CheckScreenshot(By.CLASS_NAME, 'page_generated',
 Take a full screenshot and compare with the image in
 `screenshots/full_screenshot.png` using the ignore-mask
 `screenshots/full_screenshot_mask.png`. The viewport resolution can be set on
-the SeleniumContext constructor.
+the SeleniumContext constructor. Store the temporary files
+(`full_screenshot.diff.png` and `full_screenshot.test.png`) in '~/tmp'.
 ```
 CheckScreenshot(ref_img='screenshots/full_screenshot.png',
+                output_dir='~/tmp',
                 rm_images_on_success=False,
                 mask_img='screenshots/full_screenshot_mask.png',
                 threshold=0.01),
@@ -144,7 +147,7 @@ CheckScreenshot(ref_img='screenshots/full_screenshot.png',
 Note that the test by default does not store the currently captured screenshot
 and the diff image in case of success. If you'd like to see those images, set
 `rm_images_on_success` to False, and those images will be available in the
-working directory (script's -w argument).
+output directory (script's -o argument - defaults to `/tmp`).
 
 ## Helpers
 
diff --git a/engine/tests/Functional.fuego_release_test/fuego_test.sh b/engine/tests/Functional.fuego_release_test/fuego_test.sh
index e87d6a4..05b4b7b 100755
--- a/engine/tests/Functional.fuego_release_test/fuego_test.sh
+++ b/engine/tests/Functional.fuego_release_test/fuego_test.sh
@@ -18,13 +18,10 @@ function test_build {
     echo "Cloning fuego-core from ${fuego_core_repo}:${fuego_core_branch}"
     git clone --depth 1 --single-branch --branch "${fuego_core_branch}" \
         "${fuego_core_repo}" "${fuego_release_dir}/fuego-core"
-
-    echo "Copying assets from ${TEST_HOME} to the buildzone."
-    cp -r "${TEST_HOME}/screenshots" .
 }
 
 function test_run {
-    sudo -n "${TEST_HOME}/test_run.py" "${fuego_release_dir}/fuego" -w .
+    sudo -n "${TEST_HOME}/test_run.py" "${fuego_release_dir}/fuego" -d ${TEST_HOME} -o .
     if [ "${?}" = 0 ]; then
         report "echo ok 1 fuego release test"
     else
diff --git a/engine/tests/Functional.fuego_release_test/test_run.py b/engine/tests/Functional.fuego_release_test/test_run.py
index 044c0d3..02c5f07 100755
--- a/engine/tests/Functional.fuego_release_test/test_run.py
+++ b/engine/tests/Functional.fuego_release_test/test_run.py
@@ -160,11 +160,12 @@ class CheckText(SeleniumCommand):
 
 
 class CheckScreenshot(SeleniumCommand):
-    def __init__(self, ref_img, locator=None, pattern=None, mask_img=None,
-                 diff_img=None, expected_result=True, threshold=0.0,
-                 rm_images_on_success=True):
-        def add_suffix(filename, suffix):
-            head, sep, tail = ref_img.rpartition('.')
+    def __init__(self, ref_img, output_dir='/tmp', locator=None,
+                 pattern=None, mask_img=None, expected_result=True,
+                 threshold=0.0, rm_images_on_success=True):
+        def basename_with_suffix(filename, suffix):
+            basename = os.path.basename(filename)
+            head, sep, tail = basename.rpartition('.')
             if sep:
                 return head + sep + suffix + '.' + tail
 
@@ -178,12 +179,11 @@ class CheckScreenshot(SeleniumCommand):
         self.threshold = threshold
         self.rm_images_on_success = rm_images_on_success
 
-        if diff_img is None:
-            self.diff_img_path = add_suffix(ref_img, 'diff')
-        else:
-            self.diff_img_path = diff_img
-
-        self.test_img_path = add_suffix(ref_img, 'test')
+        # Output files
+        self.diff_img_path = os.path.join(
+            output_dir, basename_with_suffix(ref_img, 'diff'))
+        self.test_img_path = os.path.join(
+            output_dir, basename_with_suffix(ref_img, 'test'))
 
     def compare_cmd_magick_v7(test_img, ref_img, mask_img, diff_img):
         """
@@ -687,6 +687,7 @@ def main():
     DEFAULT_INSTALL_SCRIPT = 'install.sh'
     DEFAULT_START_SCRIPT = 'fuego-host-scripts/docker-start-container.sh'
     DEFAULT_JENKINS_PORT = 8080
+    DEFAULT_OUTPUT_DIR = '/tmp'
 
     @atexit.register
     def cleanup():
@@ -698,12 +699,17 @@ def main():
     def get_abs_working_dirs():
         abs_install_dir = os.path.abspath(args.install_dir)
 
-        if args.working_dir:
-            abs_working_dir = os.path.abspath(args.working_dir)
+        if args.resources_dir:
+            abs_resources_dir = os.path.abspath(args.resources_dir)
+        else:
+            abs_resources_dir = abs_install_dir
+
+        if args.output_dir:
+            abs_output_dir = os.path.abspath(args.output_dir)
         else:
-            abs_working_dir = abs_install_dir
+            abs_output_dir = abs_output_dir
 
-        return abs_install_dir, abs_working_dir
+        return abs_install_dir, abs_resources_dir, abs_output_dir
 
     def execute_tests(timeout):
         LOGGER.info("Starting tests...")
@@ -733,10 +739,14 @@ def main():
     parser = argparse.ArgumentParser()
     parser.add_argument('install_dir', help="The directory where the install "
                         "script resides.", type=str)
-    parser.add_argument('-w', '--working_dir', help="The working directory. "
-                        "Location of the test assets and where test results "
-                        "will be stored. Defaults to install_dir.",
+    parser.add_argument('-d', '--resources_dir', help="Root directory for " +
+                        " test assets, as reference screenshots and other "
+                        " resources. Defaults to install_dir.",
                         default=None, type=str)
+    parser.add_argument('-o', '--output_dir', help="Location in which " +
+                        "temporary files generated by test cases will be " +
+                        "stored. Defaults to '%s'." % DEFAULT_OUTPUT_DIR,
+                        default=DEFAULT_OUTPUT_DIR, type=str)
     parser.add_argument('-s', '--install-script',
                         help="The script that will be used to install the " +
                         "docker image. Defaults to '%s'" %
@@ -770,7 +780,8 @@ def main():
                         default=True, action='store_false')
     args = parser.parse_args()
 
-    args.install_dir, args.working_dir = get_abs_working_dirs()
+    args.install_dir, args.resources_dir, args.output_dir = \
+        get_abs_working_dirs()
 
     LOGGER.debug("Changing working dir to '%s'", args.install_dir)
     os.chdir(args.install_dir)
@@ -794,9 +805,9 @@ def main():
         LOGGER.error("Selenium session failed to start")
         return 1
 
-    if os.getcwd != args.working_dir:
-        LOGGER.debug("Changing working dir to '%s'", args.working_dir)
-        os.chdir(args.working_dir)
+    if os.getcwd != args.resources_dir:
+        LOGGER.debug("Changing working dir to '%s'", args.resources_dir)
+        os.chdir(args.resources_dir)
 
     COMMANDS_TO_TEST = [
         # Set Selenium Browser root
@@ -842,12 +853,14 @@ def main():
 
         # Compare screenshot of an element of Jenkins UI
         CheckScreenshot(ref_img='screenshots/side-panel-tasks.png',
+                        output_dir=args.output_dir,
                         locator=By.ID, pattern='tasks',
                         rm_images_on_success=True,
                         threshold=0.1),
 
         # Compare screenshot of an element of Jenkins UI ignoring an area
         CheckScreenshot(ref_img='screenshots/footer.png',
+                        output_dir=args.output_dir,
                         locator=By.CLASS_NAME, pattern='col-md-18',
                         rm_images_on_success=False,
                         mask_img='screenshots/footer_mask.png',
-- 
2.17.0

[Fuego] [PATCH 3/8] Remove requirement of running the test script as sudo

[Guilherme Campos Camargo]

By requiring the user to run the `test_run.sh` script as sudo on
standalone mode (outside Fuego's docker container) we may be exposing
the user to unwanted overwrites through the `output_dir` argument -
that's currently being used on CheckScreenshot.

On this patch, we remove that requirement and also update the REAMDE.md
with better instructions (discouraging running the script with sudo).

Given that the install/start scripts may execute commands with
sudo, we're redirecting the password prompt to the users so that they
explicitly allow those scripts to execute with root privileges.

In order to check wether the start script requires root priviledges or
not, we're using a regexp on pexpect that looks for the string 'password
for' on the output of the script. That string is subject to locale
settings, so, in order to avoid problems matching the regexp, we're
standardizing the locale to english by setting LANG='en_US.UTF-8'.

Signed-off-by: Guilherme Campos Camargo <guicc@profusion.mobi>
---
 .../Functional.fuego_release_test/README.md   | 10 ++++++++--
 .../Functional.fuego_release_test/test_run.py | 20 +++++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/engine/tests/Functional.fuego_release_test/README.md b/engine/tests/Functional.fuego_release_test/README.md
index d9cb7b6..fbe4360 100644
--- a/engine/tests/Functional.fuego_release_test/README.md
+++ b/engine/tests/Functional.fuego_release_test/README.md
@@ -72,8 +72,14 @@ test_run:INFO: Waiting for jenkins on '172.17.0.2:8080'...
 
 ```
 
-Docker commands will be executed from within the script. For that reason, you
-may be required to execute it with `sudo` or as a user with root permissions.
+Docker commands will be executed from within the script, what means that
+you will need to make sure that your user is part of the `docker` group
+for running this script without root permissions.
+
+If the install or the start scripts require a password, you will be
+prompted for the it during the execution.
+
+Running this script as root is not recommended.
 
 ## Test Classes
 
diff --git a/engine/tests/Functional.fuego_release_test/test_run.py b/engine/tests/Functional.fuego_release_test/test_run.py
index 02c5f07..9b96d0d 100755
--- a/engine/tests/Functional.fuego_release_test/test_run.py
+++ b/engine/tests/Functional.fuego_release_test/test_run.py
@@ -9,6 +9,7 @@ import re
 import subprocess
 import sys
 import time
+from getpass import getpass
 from io import BytesIO
 
 import docker
@@ -592,12 +593,26 @@ class PexpectContainerSession():
         self.timeout = timeout
 
     def start(self):
+        def prompt_password_if_needed():
+            try:
+                PASSWORD_PROMPT_REGEX = re.compile("password for.+$")
+                self.client.expect(PASSWORD_PROMPT_REGEX, timeout=1)
+            except pexpect.exceptions.TIMEOUT:
+                pass
+            else:
+                prompt = self.client.before + self.client.match.group(0)
+                LOGGER.info("  Start script requires a password...")
+                password = getpass(prompt=prompt)
+                self.client.sendline(password)
+
         LOGGER.info(
             "Starting container '%s'...", self.container.container_name)
         self.client = pexpect.spawnu(
             '%s %s' % (self.start_script, self.container.container_name),
             echo=False, timeout=self.timeout)
 
+        prompt_password_if_needed()
+
         PexpectContainerSession.set_ps1(self.client)
 
         LOGGER.info("Container started with the ip '%s'",
@@ -688,6 +703,7 @@ def main():
     DEFAULT_START_SCRIPT = 'fuego-host-scripts/docker-start-container.sh'
     DEFAULT_JENKINS_PORT = 8080
     DEFAULT_OUTPUT_DIR = '/tmp'
+    DEFAULT_LOCALE = 'en_US.UTF-8'
 
     @atexit.register
     def cleanup():
@@ -786,6 +802,10 @@ def main():
     LOGGER.debug("Changing working dir to '%s'", args.install_dir)
     os.chdir(args.install_dir)
 
+    LOGGER.debug("Setting locale to '%s' to standardize Pexpect output",
+                 DEFAULT_LOCALE)
+    os.environ['LANG'] = DEFAULT_LOCALE
+
     container = FuegoContainer(args.install_script, args.image_name,
                                args.container_name, args.jenkins_port,
                                rm_after_test=args.rm_test_container)
-- 
2.17.0

[Fuego] [PATCH 4/8] Catch PermissionError when saving screenshots

[Guilherme Campos Camargo]

Also catch PermissionErrors when saving screenshot, along with
FileNotFound errors.

Signed-off-by: Guilherme Campos Camargo <guicc@profusion.mobi>
---
 engine/tests/Functional.fuego_release_test/test_run.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/engine/tests/Functional.fuego_release_test/test_run.py b/engine/tests/Functional.fuego_release_test/test_run.py
index 9b96d0d..4013378 100755
--- a/engine/tests/Functional.fuego_release_test/test_run.py
+++ b/engine/tests/Functional.fuego_release_test/test_run.py
@@ -315,10 +315,11 @@ class CheckScreenshot(SeleniumCommand):
 
         try:
             screenshot.save(self.test_img_path, format='PNG')
-        except FileNotFoundError:
+        except (FileNotFoundError, PermissionError) as e:
             LOGGER.error(
-                "  Unable to save the screenshot to '%s'. Does the directory "
-                "exist and can you write to it?", self.test_img_path)
+                "  Unable to save the screenshot to '%s'. Does the " +
+                " directory exist and can you write to it? Error: %s",
+                self.test_img_path, e)
             return False
 
         result = self.compare_images(self.test_img_path,
-- 
2.17.0

[Fuego] [PATCH 5/8] Add new log level REPORT

[Guilherme Campos Camargo]

The new log level (REPORT) will be used to report test results. It has
priority 51 (higher than CRITICAL).

Signed-off-by: Guilherme Campos Camargo <guicc@profusion.mobi>
---
 .../tests/Functional.fuego_release_test/test_run.py  | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/engine/tests/Functional.fuego_release_test/test_run.py b/engine/tests/Functional.fuego_release_test/test_run.py
index 4013378..42aa403 100755
--- a/engine/tests/Functional.fuego_release_test/test_run.py
+++ b/engine/tests/Functional.fuego_release_test/test_run.py
@@ -20,6 +20,18 @@ from selenium import webdriver
 from selenium.webdriver.common.by import By
 from PIL import Image
 
+
+class FuegoReleaseTestLogger(logging.Logger):
+    REPORT_LOG_LEVEL = 51
+
+    def report(self, msg, *args, **kwargs):
+        self.log(FuegoReleaseTestLogger.REPORT_LOG_LEVEL,
+                 msg, *args, **kwargs)
+
+
+logging.setLoggerClass(FuegoReleaseTestLogger)
+logging.addLevelName(FuegoReleaseTestLogger.REPORT_LOG_LEVEL, "REPORT")
+
 LOGGER = logging.getLogger('test_run')
 STREAM_HANDLER = logging.StreamHandler()
 STREAM_HANDLER.setFormatter(
-- 
2.17.0

[Fuego] [PATCH 6/8] Add `description` optional argument to test cases

[Guilherme Campos Camargo]

Add a `description` optional argument to test cases, that may be used by
execute_tests to print the test description on the final report.

Signed-off-by: Guilherme Campos Camargo <guicc@profusion.mobi>
---
 .../Functional.fuego_release_test/test_run.py | 22 ++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/engine/tests/Functional.fuego_release_test/test_run.py b/engine/tests/Functional.fuego_release_test/test_run.py
index 42aa403..e226a96 100755
--- a/engine/tests/Functional.fuego_release_test/test_run.py
+++ b/engine/tests/Functional.fuego_release_test/test_run.py
@@ -131,10 +131,12 @@ class SeleniumCommand:
 
 
 class Visit(SeleniumCommand):
-    def __init__(self, url, timeout=10, expected_result=200):
+    def __init__(self, url, timeout=10, expected_result=200,
+                 description="visit"):
         self.url = url
         self.timeout = timeout
         self.expected_result = expected_result
+        self.description = description
 
     def exec(self, selenium_ctx):
         super().exec(selenium_ctx)
@@ -154,11 +156,13 @@ class Visit(SeleniumCommand):
 
 
 class CheckText(SeleniumCommand):
-    def __init__(self, locator, pattern, text='', expected_result=True):
+    def __init__(self, locator, pattern, text='', expected_result=True,
+                 description="check text"):
         self.pattern = pattern
         self.locator = locator
         self.text = text
         self.expected_result = expected_result
+        self.description = description
 
     def exec(self, selenium_ctx):
         super().exec(selenium_ctx)
@@ -175,7 +179,8 @@ class CheckText(SeleniumCommand):
 class CheckScreenshot(SeleniumCommand):
     def __init__(self, ref_img, output_dir='/tmp', locator=None,
                  pattern=None, mask_img=None, expected_result=True,
-                 threshold=0.0, rm_images_on_success=True):
+                 threshold=0.0, rm_images_on_success=True,
+                 description="check screenshot"):
         def basename_with_suffix(filename, suffix):
             basename = os.path.basename(filename)
             head, sep, tail = basename.rpartition('.')
@@ -191,6 +196,7 @@ class CheckScreenshot(SeleniumCommand):
         self.expected_result = expected_result
         self.threshold = threshold
         self.rm_images_on_success = rm_images_on_success
+        self.description = description
 
         # Output files
         self.diff_img_path = os.path.join(
@@ -351,9 +357,10 @@ class CheckScreenshot(SeleniumCommand):
 
 
 class Click(SeleniumCommand):
-    def __init__(self, locator, pattern):
+    def __init__(self, locator, pattern, description="click"):
         self.pattern = pattern
         self.locator = locator
+        self.description = description
 
     def exec(self, selenium_ctx):
         super().exec(selenium_ctx)
@@ -367,6 +374,9 @@ class Click(SeleniumCommand):
 
 
 class Back(SeleniumCommand):
+    def __init__(self, description="back"):
+        self.description = description
+
     def exec(self, selenium_ctx):
         super().exec(selenium_ctx)
 
@@ -386,10 +396,12 @@ class ShExpect():
         (COMMAND_OUTPUT_DELIM, COMMAND_OUTPUT_DELIM, BASH_PATTERN),
         re.M | re.S)
 
-    def __init__(self, cmd, expected_output=None, expected_result=0):
+    def __init__(self, cmd, expected_output=None, expected_result=0,
+                 description=None):
         self.cmd = cmd
         self.expected_result = expected_result
         self.expected_output = expected_output
+        self.description = description or "sh expect %s" % cmd
 
     def exec(self, pexpect_ctx):
         self.client = pexpect_ctx.client
-- 
2.17.0

[Fuego] [PATCH 7/8] Add description to tests in COMMANDS_TO_TEST

[Guilherme Campos Camargo]

Add a description for most of the tests.

Signed-off-by: Guilherme Campos Camargo <guicc@profusion.mobi>
---
 .../Functional.fuego_release_test/test_run.py | 36 ++++++++++++-------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/engine/tests/Functional.fuego_release_test/test_run.py b/engine/tests/Functional.fuego_release_test/test_run.py
index e226a96..52fbae3 100755
--- a/engine/tests/Functional.fuego_release_test/test_run.py
+++ b/engine/tests/Functional.fuego_release_test/test_run.py
@@ -856,52 +856,63 @@ def main():
 
     COMMANDS_TO_TEST = [
         # Set Selenium Browser root
-        Visit(url=container.get_url()),
+        Visit(url=container.get_url(),
+              description="visit " + container.get_url()),
 
         # Compare screenshot of the full viewport ignoring an area
         CheckScreenshot(ref_img='screenshots/full_screenshot.png',
                         rm_images_on_success=False,
                         mask_img='screenshots/full_screenshot_mask.png',
-                        threshold=0.05),
+                        threshold=0.05,
+                        description="check full screenshot"),
 
         # Add Nodes
         ShExpect('ftc add-nodes docker'),
         ShExpect('ftc list-nodes -q', r'.*docker.*'),
-        CheckText(By.ID, 'executors', text='master'),
-        CheckText(By.ID, 'executors', text='docker'),
+        CheckText(By.ID, 'executors', text='master',
+                  description="check text executors master"),
+        CheckText(By.ID, 'executors', text='docker',
+                  description="check text executors docker"),
 
         # Add Fuego TestPlan
         ShExpect('ftc add-jobs -b docker -p testplan_fuego_tests'),
         ShExpect('ftc list-jobs', r'.*docker\.testplan_fuego_tests\.batch.*'),
 
-        Click(By.PARTIAL_LINK_TEXT, 'docker'),
+        Click(By.PARTIAL_LINK_TEXT, 'docker',
+              description="click docker"),
         CheckText(By.ID, 'projectstatus',
-                  text='docker.testplan_fuego_tests.batch'),
+                  text='docker.testplan_fuego_tests.batch',
+                  description="check text projectstatus fuego batch"),
         Back(),
 
         # Install Views
         ShExpect('ftc add-view batch .*.batch'),
         CheckText(By.ID, 'projectstatus-tabBar',
-                  text='batch'),
+                  text='batch',
+                  description="check text projectstatus tab bar batch"),
 
         # Start a Test through Command Line
         ShExpect('ftc build-jobs *.*.Functional.fuego_board_check'),
         CheckText(By.ID, 'buildQueue',
-                  text='Functional.fuego_board_check'),
+                  text='Functional.fuego_board_check',
+                  description="check text build queue fuego board check"),
 
         # Start a Test through Jenkins UI (Xpath)
         Click(By.XPATH,
               '//img[@title="Schedule a build for ' +
-              'docker.default.Functional.hello_world"]'),
+              'docker.default.Functional.hello_world"]',
+              description="click img build hello world"),
         CheckText(By.ID, 'executors',
-                  text='docker.default.Functional.hello_world'),
+                  text='docker.default.Functional.hello_world',
+                  description="check text executors hello world"),
 
         # Compare screenshot of an element of Jenkins UI
         CheckScreenshot(ref_img='screenshots/side-panel-tasks.png',
                         output_dir=args.output_dir,
                         locator=By.ID, pattern='tasks',
                         rm_images_on_success=True,
-                        threshold=0.1),
+                        threshold=0.1,
+                        description="check screenshot side-panel-tasks"),
 
         # Compare screenshot of an element of Jenkins UI ignoring an area
         CheckScreenshot(ref_img='screenshots/footer.png',
@@ -909,7 +920,8 @@ def main():
                         locator=By.CLASS_NAME, pattern='col-md-18',
                         rm_images_on_success=False,
                         mask_img='screenshots/footer_mask.png',
-                        threshold=0.1),
+                        threshold=0.1,
+                        description="check screenshot footer"),
     ]
 
     if not execute_tests(args.timeout):
-- 
2.17.0

[Fuego] [PATCH 8/8] Log test results with the REPORT log level and print descriptions

[Guilherme Campos Camargo]

Log test results with the REPORT log level and print descriptions. In
this patch we also update fuego_test.sh to allow parsing the new test
results format.

Signed-off-by: Guilherme Campos Camargo <guicc@profusion.mobi>
---
 .../tests/Functional.fuego_release_test/fuego_test.sh  | 10 +++-------
 engine/tests/Functional.fuego_release_test/test_run.py |  7 ++++---
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/engine/tests/Functional.fuego_release_test/fuego_test.sh b/engine/tests/Functional.fuego_release_test/fuego_test.sh
index 05b4b7b..242fa70 100755
--- a/engine/tests/Functional.fuego_release_test/fuego_test.sh
+++ b/engine/tests/Functional.fuego_release_test/fuego_test.sh
@@ -21,14 +21,10 @@ function test_build {
 }
 
 function test_run {
-    sudo -n "${TEST_HOME}/test_run.py" "${fuego_release_dir}/fuego" -d ${TEST_HOME} -o .
-    if [ "${?}" = 0 ]; then
-        report "echo ok 1 fuego release test"
-    else
-        report "echo not ok 1 fuego release test"
-    fi
+    report "sudo -n ${TEST_HOME}/test_run.py ${fuego_release_dir}/fuego -d ${TEST_HOME} -o ."
 }
 
 function test_processing {
-    log_compare "$TESTDIR" "1" "^ok" "p"
+    log_compare "$TESTDIR" "1" "ok" "p"
+    log_compare "$TESTDIR" "0" "not ok" "n"
 }
diff --git a/engine/tests/Functional.fuego_release_test/test_run.py b/engine/tests/Functional.fuego_release_test/test_run.py
index 52fbae3..d3c65f6 100755
--- a/engine/tests/Functional.fuego_release_test/test_run.py
+++ b/engine/tests/Functional.fuego_release_test/test_run.py
@@ -761,14 +761,15 @@ def main():
         }
 
         tests_ok = True
-        for cmd in COMMANDS_TO_TEST:
+        for cmd_id, cmd in enumerate(COMMANDS_TO_TEST, 1):
             for base_class, ctx in ctx_mapper.items():
                 if isinstance(cmd, base_class):
+                    LOGGER.report("run %s %s", cmd_id, cmd.description)
                     if not cmd.exec(ctx):
                         tests_ok = False
-                        LOGGER.error("  FAIL")
+                        LOGGER.report("not ok %s", cmd_id)
                         break
-                    LOGGER.info("  PASS")
+                    LOGGER.report("ok %s", cmd_id)
 
         if tests_ok:
             LOGGER.info("All tests finished with SUCCESS")
-- 
2.17.0

Re: [Fuego] [PATCH 8/8] Log test results with the REPORT log level and print descriptions

[Tim.Bird]

> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> bounces@lists.linuxfoundation.org] On Behalf Of Guilherme Campos
> Camargo
> Sent: Wednesday, May 02, 2018 7:21 AM
> To: fuego@lists.linuxfoundation.org
> Subject: [Fuego] [PATCH 8/8] Log test results with the REPORT log level and
> print descriptions
> 
> Log test results with the REPORT log level and print descriptions. In
> this patch we also update fuego_test.sh to allow parsing the new test
> results format.
> 
> Signed-off-by: Guilherme Campos Camargo <guicc@profusion.mobi>
> ---
>  .../tests/Functional.fuego_release_test/fuego_test.sh  | 10 +++-------
>  engine/tests/Functional.fuego_release_test/test_run.py |  7 ++++---
>  2 files changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/engine/tests/Functional.fuego_release_test/fuego_test.sh
> b/engine/tests/Functional.fuego_release_test/fuego_test.sh
> index 05b4b7b..242fa70 100755
> --- a/engine/tests/Functional.fuego_release_test/fuego_test.sh
> +++ b/engine/tests/Functional.fuego_release_test/fuego_test.sh
> @@ -21,14 +21,10 @@ function test_build {
>  }
> 
>  function test_run {
> -    sudo -n "${TEST_HOME}/test_run.py" "${fuego_release_dir}/fuego" -d
> ${TEST_HOME} -o .
> -    if [ "${?}" = 0 ]; then
> -        report "echo ok 1 fuego release test"
> -    else
> -        report "echo not ok 1 fuego release test"
> -    fi
> +    report "sudo -n ${TEST_HOME}/test_run.py ${fuego_release_dir}/fuego -
> d ${TEST_HOME} -o ."
>  }
> 
>  function test_processing {
> -    log_compare "$TESTDIR" "1" "^ok" "p"
> +    log_compare "$TESTDIR" "1" "ok" "p"

I suspect this should be more than 1.  How many reports are generated with "ok"

Also, I think more context would be good, to avoid false matches.

Maybe log_compare "$TESTDIR" "19" "test_run:REPORT: ok"

> +    log_compare "$TESTDIR" "0" "not ok" "n"
I'm not sure this is needed, but it does future-proof the test a bit.

>  }
> diff --git a/engine/tests/Functional.fuego_release_test/test_run.py
> b/engine/tests/Functional.fuego_release_test/test_run.py
> index 52fbae3..d3c65f6 100755
> --- a/engine/tests/Functional.fuego_release_test/test_run.py
> +++ b/engine/tests/Functional.fuego_release_test/test_run.py
> @@ -761,14 +761,15 @@ def main():
>          }
> 
>          tests_ok = True
> -        for cmd in COMMANDS_TO_TEST:
> +        for cmd_id, cmd in enumerate(COMMANDS_TO_TEST, 1):
>              for base_class, ctx in ctx_mapper.items():
>                  if isinstance(cmd, base_class):
> +                    LOGGER.report("run %s %s", cmd_id, cmd.description)
>                      if not cmd.exec(ctx):
>                          tests_ok = False
> -                        LOGGER.error("  FAIL")
> +                        LOGGER.report("not ok %s", cmd_id)
>                          break
> -                    LOGGER.info("  PASS")
> +                    LOGGER.report("ok %s", cmd_id)
> 
>          if tests_ok:
>              LOGGER.info("All tests finished with SUCCESS")
> --
> 2.17.0

Thanks,
  -- Tim

Re: [Fuego] [PATCH 0/8] fuego-release-test fixes

[Tim.Bird]

This looks nice.  This test will be now be a candidate for our
per-testcase parsing.   See the parser.py for curl as an example.

Thanks.

All patches in this series were applied and pushed.
 -- Tim


> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> bounces@lists.linuxfoundation.org] On Behalf Of Guilherme Campos
> Camargo
> Sent: Wednesday, May 02, 2018 7:21 AM
> To: fuego@lists.linuxfoundation.org
> Subject: [Fuego] [PATCH 0/8] fuego-release-test fixes
> 
> Hello, Tim
> 
> These patches are implementing the changes you've requested in your
> review and a few other fixes, in summary:
> 
>   - Replace working_dir with two separate dirs: resources_dir (for test
>     assets) and output_dir (for test resulting files)
> 
>   - Print the results of the tests individually, prefixing them with an
>     id and with a summary - and parse them individually with log_compare
>     ("ok" and "not ok")
> 
>     Output Example (Log level set as INFO)
> 
>     ```
>     ...
>     test_run:REPORT: run 15 check text build queue fuego board check
>     test_run:INFO: Executing Selenium Command 'CheckText'
>     test_run:INFO:   Matching text found
>     test_run:REPORT: ok 15
>     test_run:REPORT: run 16 click img build hello world
>     test_run:INFO: Executing Selenium Command 'Click'
>     test_run:INFO:   Element clicked
>     test_run:REPORT: ok 16
>     test_run:REPORT: run 17 check text executors hello world
>     test_run:INFO: Executing Selenium Command 'CheckText'
>     test_run:INFO:   Matching text found
>     test_run:REPORT: ok 17
>     test_run:REPORT: run 18 check screenshot side-panel-tasks
>     test_run:INFO: Executing Selenium Command 'CheckScreenshot'
>     test_run:INFO:   Resulting difference (0.00123834) below threshold (0.1)
>     test_run:INFO:   Element's screenshot matches the reference
>     test_run:REPORT: ok 18
>     test_run:REPORT: run 19 check screenshot footer
>     test_run:INFO: Executing Selenium Command 'CheckScreenshot'
>     test_run:INFO:   Resulting difference (0.0) below threshold (0.1)
>     test_run:INFO:   Element's screenshot matches the reference
>     test_run:REPORT: ok 19
>     test_run:INFO: All tests finished with SUCCESS
>     ```
> 
>     Output Example (Log level set as REPORT)
> 
>     ```
>     ...
>     test_run:REPORT: run 15 check text build queue fuego board check
>     test_run:REPORT: ok 15
>     test_run:REPORT: run 16 click img build hello world
>     test_run:REPORT: ok 16
>     test_run:REPORT: run 17 check text executors hello world
>     test_run:REPORT: ok 17
>     test_run:REPORT: run 18 check screenshot side-panel-tasks
>     test_run:REPORT: ok 18
>     test_run:REPORT: run 19 check screenshot footer
>     test_run:REPORT: ok 19
> 
>   - Do not require running the test with sudo - See the commit message
>     `Remove requirement of running the test script as sudo`
> 
>   - Implement a few other minor fixes as updating the readme and catch
>     an PermissionError on image.save().
> 
> Please let me know what you think.
> 
> Thanks
> 
> --
> Guilherme
> 
> Guilherme Campos Camargo (8):
>   Update ip example of README.md
>   Replace working_dir arg with resources_dir and output_dir
>   Remove requirement of running the test script as sudo
>   Catch PermissionError when saving screenshots
>   Add new log level REPORT
>   Add `description` optional argument to test cases
>   Add description to tests in COMMANDS_TO_TEST
>   Log test results with the REPORT log level and print descriptions
> 
>  .../Functional.fuego_release_test/README.md   |  45 ++---
>  .../fuego_test.sh                             |  13 +-
>  .../Functional.fuego_release_test/test_run.py | 159 +++++++++++++-----
>  3 files changed, 144 insertions(+), 73 deletions(-)
> 
> --
> 2.17.0
> 
> _______________________________________________
> Fuego mailing list
> Fuego@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/fuego

Re: [Fuego] [PATCH 3/8] Remove requirement of running the test script as sudo

[Tim.Bird]

I ran the test on my machine without issues, and without any
apparent prompt for a password.  Under what conditions
is a password prompt required?  I'm not sure I'm following the code.

 -- Tim

> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> bounces@lists.linuxfoundation.org] On Behalf Of Guilherme Campos
> Camargo
> Sent: Wednesday, May 02, 2018 7:21 AM
> To: fuego@lists.linuxfoundation.org
> Subject: [Fuego] [PATCH 3/8] Remove requirement of running the test script
> as sudo
> 
> By requiring the user to run the `test_run.sh` script as sudo on
> standalone mode (outside Fuego's docker container) we may be exposing
> the user to unwanted overwrites through the `output_dir` argument -
> that's currently being used on CheckScreenshot.
> 
> On this patch, we remove that requirement and also update the REAMDE.md
> with better instructions (discouraging running the script with sudo).
> 
> Given that the install/start scripts may execute commands with
> sudo, we're redirecting the password prompt to the users so that they
> explicitly allow those scripts to execute with root privileges.
> 
> In order to check wether the start script requires root priviledges or
> not, we're using a regexp on pexpect that looks for the string 'password
> for' on the output of the script. That string is subject to locale
> settings, so, in order to avoid problems matching the regexp, we're
> standardizing the locale to english by setting LANG='en_US.UTF-8'.
> 
> Signed-off-by: Guilherme Campos Camargo <guicc@profusion.mobi>
> ---
>  .../Functional.fuego_release_test/README.md   | 10 ++++++++--
>  .../Functional.fuego_release_test/test_run.py | 20 +++++++++++++++++++
>  2 files changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/engine/tests/Functional.fuego_release_test/README.md
> b/engine/tests/Functional.fuego_release_test/README.md
> index d9cb7b6..fbe4360 100644
> --- a/engine/tests/Functional.fuego_release_test/README.md
> +++ b/engine/tests/Functional.fuego_release_test/README.md
> @@ -72,8 +72,14 @@ test_run:INFO: Waiting for jenkins on
> '172.17.0.2:8080'...
> 
>  ```
> 
> -Docker commands will be executed from within the script. For that reason,
> you
> -may be required to execute it with `sudo` or as a user with root permissions.
> +Docker commands will be executed from within the script, what means that
> +you will need to make sure that your user is part of the `docker` group
> +for running this script without root permissions.
> +
> +If the install or the start scripts require a password, you will be
> +prompted for the it during the execution.
> +
> +Running this script as root is not recommended.
> 
>  ## Test Classes
> 
> diff --git a/engine/tests/Functional.fuego_release_test/test_run.py
> b/engine/tests/Functional.fuego_release_test/test_run.py
> index 02c5f07..9b96d0d 100755
> --- a/engine/tests/Functional.fuego_release_test/test_run.py
> +++ b/engine/tests/Functional.fuego_release_test/test_run.py
> @@ -9,6 +9,7 @@ import re
>  import subprocess
>  import sys
>  import time
> +from getpass import getpass
>  from io import BytesIO
> 
>  import docker
> @@ -592,12 +593,26 @@ class PexpectContainerSession():
>          self.timeout = timeout
> 
>      def start(self):
> +        def prompt_password_if_needed():
> +            try:
> +                PASSWORD_PROMPT_REGEX = re.compile("password for.+$")
> +                self.client.expect(PASSWORD_PROMPT_REGEX, timeout=1)
> +            except pexpect.exceptions.TIMEOUT:
> +                pass
> +            else:
> +                prompt = self.client.before + self.client.match.group(0)
> +                LOGGER.info("  Start script requires a password...")
> +                password = getpass(prompt=prompt)
> +                self.client.sendline(password)
> +
>          LOGGER.info(
>              "Starting container '%s'...", self.container.container_name)
>          self.client = pexpect.spawnu(
>              '%s %s' % (self.start_script, self.container.container_name),
>              echo=False, timeout=self.timeout)
> 
> +        prompt_password_if_needed()
> +
>          PexpectContainerSession.set_ps1(self.client)
> 
>          LOGGER.info("Container started with the ip '%s'",
> @@ -688,6 +703,7 @@ def main():
>      DEFAULT_START_SCRIPT = 'fuego-host-scripts/docker-start-container.sh'
>      DEFAULT_JENKINS_PORT = 8080
>      DEFAULT_OUTPUT_DIR = '/tmp'
> +    DEFAULT_LOCALE = 'en_US.UTF-8'
> 
>      @atexit.register
>      def cleanup():
> @@ -786,6 +802,10 @@ def main():
>      LOGGER.debug("Changing working dir to '%s'", args.install_dir)
>      os.chdir(args.install_dir)
> 
> +    LOGGER.debug("Setting locale to '%s' to standardize Pexpect output",
> +                 DEFAULT_LOCALE)
> +    os.environ['LANG'] = DEFAULT_LOCALE
> +
>      container = FuegoContainer(args.install_script, args.image_name,
>                                 args.container_name, args.jenkins_port,
>                                 rm_after_test=args.rm_test_container)
> --
> 2.17.0
> 
> _______________________________________________
> Fuego mailing list
> Fuego@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/fuego

Re: [Fuego] [PATCH 8/8] Log test results with the REPORT log level and print descriptions

[Guilherme Camargo]

[-- Attachment #1: Type: text/plain, Size: 3858 bytes --]

​Hello, Tim.

​
I wanted to avoid setting the number of expected OKs to avoid
mismatches between that number and the real number of tests
that are present on COMMANDS_TO_TEST - what may change frequently
in the near future.

Maybe the best would be simply to keep only the log_compare for
the `not ok`? Catching in case any one of the tests fails?

An I agree we could use `test_run:REPORT: ok` or
`test_run:REPORT: not ok` to give more context.

Thanks


On Fri, May 4, 2018 at 5:24 PM, <Tim.Bird@sony.com> wrote:

>
>
> > -----Original Message-----
> > From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> > bounces@lists.linuxfoundation.org] On Behalf Of Guilherme Campos
> > Camargo
> > Sent: Wednesday, May 02, 2018 7:21 AM
> > To: fuego@lists.linuxfoundation.org
> > Subject: [Fuego] [PATCH 8/8] Log test results with the REPORT log level
> and
> > print descriptions
> >
> > Log test results with the REPORT log level and print descriptions. In
> > this patch we also update fuego_test.sh to allow parsing the new test
> > results format.
> >
> > Signed-off-by: Guilherme Campos Camargo <guicc@profusion.mobi>
> > ---
> >  .../tests/Functional.fuego_release_test/fuego_test.sh  | 10 +++-------
> >  engine/tests/Functional.fuego_release_test/test_run.py |  7 ++++---
> >  2 files changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/engine/tests/Functional.fuego_release_test/fuego_test.sh
> > b/engine/tests/Functional.fuego_release_test/fuego_test.sh
> > index 05b4b7b..242fa70 100755
> > --- a/engine/tests/Functional.fuego_release_test/fuego_test.sh
> > +++ b/engine/tests/Functional.fuego_release_test/fuego_test.sh
> > @@ -21,14 +21,10 @@ function test_build {
> >  }
> >
> >  function test_run {
> > -    sudo -n "${TEST_HOME}/test_run.py" "${fuego_release_dir}/fuego" -d
> > ${TEST_HOME} -o .
> > -    if [ "${?}" = 0 ]; then
> > -        report "echo ok 1 fuego release test"
> > -    else
> > -        report "echo not ok 1 fuego release test"
> > -    fi
> > +    report "sudo -n ${TEST_HOME}/test_run.py ${fuego_release_dir}/fuego
> -
> > d ${TEST_HOME} -o ."
> >  }
> >
> >  function test_processing {
> > -    log_compare "$TESTDIR" "1" "^ok" "p"
> > +    log_compare "$TESTDIR" "1" "ok" "p"
>
> I suspect this should be more than 1.  How many reports are generated with
> "ok"
>
> Also, I think more context would be good, to avoid false matches.
>
> Maybe log_compare "$TESTDIR" "19" "test_run:REPORT: ok"
>
> > +    log_compare "$TESTDIR" "0" "not ok" "n"
> I'm not sure this is needed, but it does future-proof the test a bit.
>
> >  }
> > diff --git a/engine/tests/Functional.fuego_release_test/test_run.py
> > b/engine/tests/Functional.fuego_release_test/test_run.py
> > index 52fbae3..d3c65f6 100755
> > --- a/engine/tests/Functional.fuego_release_test/test_run.py
> > +++ b/engine/tests/Functional.fuego_release_test/test_run.py
> > @@ -761,14 +761,15 @@ def main():
> >          }
> >
> >          tests_ok = True
> > -        for cmd in COMMANDS_TO_TEST:
> > +        for cmd_id, cmd in enumerate(COMMANDS_TO_TEST, 1):
> >              for base_class, ctx in ctx_mapper.items():
> >                  if isinstance(cmd, base_class):
> > +                    LOGGER.report("run %s %s", cmd_id, cmd.description)
> >                      if not cmd.exec(ctx):
> >                          tests_ok = False
> > -                        LOGGER.error("  FAIL")
> > +                        LOGGER.report("not ok %s", cmd_id)
> >                          break
> > -                    LOGGER.info("  PASS")
> > +                    LOGGER.report("ok %s", cmd_id)
> >
> >          if tests_ok:
> >              LOGGER.info("All tests finished with SUCCESS")
> > --
> > 2.17.0
>
> Thanks,
>   -- Tim
>
>

[-- Attachment #2: Type: text/html, Size: 9419 bytes --]

Re: [Fuego] [PATCH 3/8] Remove requirement of running the test script as sudo

[Guilherme Camargo]

[-- Attachment #1: Type: text/plain, Size: 6387 bytes --]

​If you run the test from within a Fuego container, you should not be asked
for any password. It will go smoothly​ without asking anything.

However, if you run the test in standalone mode (as explained in the README
- not sure if that's how you tested in your machine), you might be asked
for a
sudo password when the test starts building (run install.sh) and when the
test calls `host-scripts/docker-create-container.sh` through pexpect.

If you're running on standalone mode, and still not being asked for the
sudo password, maybe your previous sudo session has not expired yet
(defaults to 5 minutes),
or your user is configured to be a sudoer that does not require a password?

On Fri, May 4, 2018 at 5:36 PM, <Tim.Bird@sony.com> wrote:

> I ran the test on my machine without issues, and without any
> apparent prompt for a password.  Under what conditions
> is a password prompt required?  I'm not sure I'm following the code.
>
>  -- Tim
>
> > -----Original Message-----
> > From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> > bounces@lists.linuxfoundation.org] On Behalf Of Guilherme Campos
> > Camargo
> > Sent: Wednesday, May 02, 2018 7:21 AM
> > To: fuego@lists.linuxfoundation.org
> > Subject: [Fuego] [PATCH 3/8] Remove requirement of running the test
> script
> > as sudo
> >
> > By requiring the user to run the `test_run.sh` script as sudo on
> > standalone mode (outside Fuego's docker container) we may be exposing
> > the user to unwanted overwrites through the `output_dir` argument -
> > that's currently being used on CheckScreenshot.
> >
> > On this patch, we remove that requirement and also update the REAMDE.md
> > with better instructions (discouraging running the script with sudo).
> >
> > Given that the install/start scripts may execute commands with
> > sudo, we're redirecting the password prompt to the users so that they
> > explicitly allow those scripts to execute with root privileges.
> >
> > In order to check wether the start script requires root priviledges or
> > not, we're using a regexp on pexpect that looks for the string 'password
> > for' on the output of the script. That string is subject to locale
> > settings, so, in order to avoid problems matching the regexp, we're
> > standardizing the locale to english by setting LANG='en_US.UTF-8'.
> >
> > Signed-off-by: Guilherme Campos Camargo <guicc@profusion.mobi>
> > ---
> >  .../Functional.fuego_release_test/README.md   | 10 ++++++++--
> >  .../Functional.fuego_release_test/test_run.py | 20 +++++++++++++++++++
> >  2 files changed, 28 insertions(+), 2 deletions(-)
> >
> > diff --git a/engine/tests/Functional.fuego_release_test/README.md
> > b/engine/tests/Functional.fuego_release_test/README.md
> > index d9cb7b6..fbe4360 100644
> > --- a/engine/tests/Functional.fuego_release_test/README.md
> > +++ b/engine/tests/Functional.fuego_release_test/README.md
> > @@ -72,8 +72,14 @@ test_run:INFO: Waiting for jenkins on
> > '172.17.0.2:8080'...
> >
> >  ```
> >
> > -Docker commands will be executed from within the script. For that
> reason,
> > you
> > -may be required to execute it with `sudo` or as a user with root
> permissions.
> > +Docker commands will be executed from within the script, what means that
> > +you will need to make sure that your user is part of the `docker` group
> > +for running this script without root permissions.
> > +
> > +If the install or the start scripts require a password, you will be
> > +prompted for the it during the execution.
> > +
> > +Running this script as root is not recommended.
> >
> >  ## Test Classes
> >
> > diff --git a/engine/tests/Functional.fuego_release_test/test_run.py
> > b/engine/tests/Functional.fuego_release_test/test_run.py
> > index 02c5f07..9b96d0d 100755
> > --- a/engine/tests/Functional.fuego_release_test/test_run.py
> > +++ b/engine/tests/Functional.fuego_release_test/test_run.py
> > @@ -9,6 +9,7 @@ import re
> >  import subprocess
> >  import sys
> >  import time
> > +from getpass import getpass
> >  from io import BytesIO
> >
> >  import docker
> > @@ -592,12 +593,26 @@ class PexpectContainerSession():
> >          self.timeout = timeout
> >
> >      def start(self):
> > +        def prompt_password_if_needed():
> > +            try:
> > +                PASSWORD_PROMPT_REGEX = re.compile("password for.+$")
> > +                self.client.expect(PASSWORD_PROMPT_REGEX, timeout=1)
> > +            except pexpect.exceptions.TIMEOUT:
> > +                pass
> > +            else:
> > +                prompt = self.client.before + self.client.match.group(0)
> > +                LOGGER.info("  Start script requires a password...")
> > +                password = getpass(prompt=prompt)
> > +                self.client.sendline(password)
> > +
> >          LOGGER.info(
> >              "Starting container '%s'...", self.container.container_name)
> >          self.client = pexpect.spawnu(
> >              '%s %s' % (self.start_script, self.container.container_name)
> ,
> >              echo=False, timeout=self.timeout)
> >
> > +        prompt_password_if_needed()
> > +
> >          PexpectContainerSession.set_ps1(self.client)
> >
> >          LOGGER.info("Container started with the ip '%s'",
> > @@ -688,6 +703,7 @@ def main():
> >      DEFAULT_START_SCRIPT = 'fuego-host-scripts/docker-sta
> rt-container.sh'
> >      DEFAULT_JENKINS_PORT = 8080
> >      DEFAULT_OUTPUT_DIR = '/tmp'
> > +    DEFAULT_LOCALE = 'en_US.UTF-8'
> >
> >      @atexit.register
> >      def cleanup():
> > @@ -786,6 +802,10 @@ def main():
> >      LOGGER.debug("Changing working dir to '%s'", args.install_dir)
> >      os.chdir(args.install_dir)
> >
> > +    LOGGER.debug("Setting locale to '%s' to standardize Pexpect output",
> > +                 DEFAULT_LOCALE)
> > +    os.environ['LANG'] = DEFAULT_LOCALE
> > +
> >      container = FuegoContainer(args.install_script, args.image_name,
> >                                 args.container_name, args.jenkins_port,
> >                                 rm_after_test=args.rm_test_container)
> > --
> > 2.17.0
> >
> > _______________________________________________
> > Fuego mailing list
> > Fuego@lists.linuxfoundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/fuego
>

[-- Attachment #2: Type: text/html, Size: 9451 bytes --]