[Notes] [Git][BuildStream/buildstream][tpollard/829] 6 commits: Support

Tom Pollard pushed to branch tpollard/829 at BuildStream / buildstream

Commits:

f29a0995

by Tristan Van Berkom at 2019-01-09T19:51:14Z

Support running test environments in parallel with `detox`

This patch namespaces the test temp directory and the output
coverage report file with the name of the environment under test,
such that separately run tests do not access the same files.

When running tests without tox, directly through setup.py,
then the tmp directory will still be `./tmp`.

  * .gitignore: Added new .coverage-reports/ directory

  * .gitlab-ci.yml: Rely on tox to combine and report coverage, only
    tell tox about the COVERAGE_PREFIX so that results can be namespaced
    by CI job name.

    This change also publishes the sources and final combined `.coverage`
    file in an output gitlab artifact for inspection, and lists some missing
    dependencies to the `coverage` job.

  * tox.ini: Add comments and refactor main [testenv] section so that
    other environments dont inherit too much unrelated cruft.

    Generate the coverate reports in the respective {envtmpdir} so that
    all per-process coverage files are prefixed with a full path, ensuring
    that concurrent runs don't mix reports and addressing concerns
    raised in #844.

    Also implemented new `tox -e coverage` environment to combine
    any found coverage and print a report.

  * .coveragerc: Omit .tox/ directory from coverage stats

Fixes issue #844

d364ad02

by Tristan Van Berkom at 2019-01-09T19:52:22Z

CONTRIBUTING.rst: Updated to mention new `coverage` tox environment.

Also point out that it is possible to run test environments in
parallel using the `detox` tool.

1e352434

by Tristan Van Berkom at 2019-01-09T20:36:06Z

Merge branch 'tristan/detox-tests' into 'master'

Allow using detox, and fix/refactor collection of coverage reports

Closes #844

See merge request BuildStream/buildstream!1051

116c1070

by Tristan Van Berkom at 2019-01-09T20:39:15Z

.coveragerc: Omit some things which are irrelevant to cover.

  * Omit versioneer's _version.py

  * Omit our __main__.py which is used only internally for generating
    documentation

c8f3616d

by Tristan Van Berkom at 2019-01-09T21:12:29Z

Merge branch 'tristan/refine-coverage' into 'master'

.coveragerc: Omit some things which are irrelevant to cover.

See merge request BuildStream/buildstream!1052

71189577

by Tom Pollard at 2019-01-10T10:50:38Z

WIP: Download buildtrees on demand for bst shell --use-buildtree

Changes:

.coveragerc

@@ -4,11 +4,15 @@ include =
    */buildstream/*
  omit =
 -  # Omit profiling helper module
 +  # Omit some internals
    */buildstream/_profile.py
 +  */buildstream/__main__.py
 +  */buildstream/_version.py
    # Omit generated code
    */buildstream/_protos/*
    */.eggs/*
 +  # Omit .tox directory
 +  */.tox/*
  [report]
  show_missing = True

.gitignore

@@ -13,11 +13,12 @@ tests/**/*.pyc
  integration-cache/
  tmp
  .coverage
 +.coverage-reports/
  .coverage.*
  .cache
  .pytest_cache/
  *.bst/
 -.tox
 +.tox/
  # Pycache, in case buildstream is ran directly from within the source
  # tree

.gitlab-ci.yml

@@ -13,6 +13,7 @@ variables:
    PYTEST_ADDOPTS: "--color=yes"
    INTEGRATION_CACHE: "${CI_PROJECT_DIR}/cache/integration-cache"
    TEST_COMMAND: "tox -- --color=yes --integration"
 +  COVERAGE_PREFIX: "${CI_JOB_NAME}."
  #####################################################
@@ -24,9 +25,6 @@ variables:
  .tests-template: &tests
    stage: test
 -  variables:
 -    COVERAGE_DIR: coverage-linux
+-
    before_script:
    # Diagnostics
    - mount
@@ -40,14 +38,11 @@ variables:
    - su buildstream -c "${TEST_COMMAND}"
    after_script:
 -  # Collect our reports
 -  - mkdir -p ${COVERAGE_DIR}
 -  - cp .coverage ${COVERAGE_DIR}/coverage."${CI_JOB_NAME}"
    except:
    - schedules
    artifacts:
      paths:
 -    - ${COVERAGE_DIR}
 +    - .coverage-reports
  tests-debian-9:
    image: buildstream/testsuite-debian:9-5da27168-32c47d1c
@@ -83,7 +78,6 @@ tests-unix:
    <<: *tests
    variables:
      BST_FORCE_BACKEND: "unix"
 -    COVERAGE_DIR: coverage-unix
    script:
@@ -239,22 +233,22 @@ coverage:
    stage: post
    coverage: '/TOTAL +\d+ +\d+ +(\d+\.\d+)%/'
    script:
 -    - pip3 install -r requirements/requirements.txt -r requirements/dev-requirements.txt
 -    - pip3 install --no-index .
 -    - mkdir report
 -    - cd report
 -    - cp ../coverage-unix/coverage.* .
 -    - cp ../coverage-linux/coverage.* .
 -    - ls coverage.*
 -    - coverage combine --rcfile=../.coveragerc -a coverage.*
 -    - coverage report --rcfile=../.coveragerc -m
 +    - cp -a .coverage-reports/ ./coverage-sources
 +    - tox -e coverage
 +    - cp -a .coverage-reports/ ./coverage-report
    dependencies:
    - tests-debian-9
    - tests-fedora-27
    - tests-fedora-28
 +  - tests-fedora-missing-deps
 +  - tests-ubuntu-18.04
    - tests-unix
    except:
    - schedules
 +  artifacts:
 +    paths:
 +    - coverage-sources/
 +    - coverage-report/
  # Deploy, only for merges which land on master branch.
+ #

CONTRIBUTING.rst

@@ -1498,6 +1498,13 @@ option when running tox::
    tox -e py37
 +If you would like to test and lint at the same time, or if you do have multiple
 +python versions installed and would like to test against multiple versions, then
 +we recommend using `detox <https://github.com/tox-dev/detox>`_, just run it with
 +the same arguments you would give `tox`::
++
 +  detox -e lint,py36,py37
++
  Linting is performed separately from testing. In order to run the linting step which
  consists of running the ``pycodestyle`` and ``pylint`` tools, run the following::
@@ -1574,6 +1581,19 @@ can run ``tox`` with ``-r`` or  ``--recreate`` option.
       ./setup.py test --addopts 'tests/frontend/buildtrack.py::test_build_track'
 +Observing coverage
 +~~~~~~~~~~~~~~~~~~
 +Once you have run the tests using `tox` (or `detox`), some coverage reports will
 +have been left behind.
++
 +To view the coverage report of the last test run, simply run::
++
 +  tox -e coverage
++
 +This will collate any reports from separate python environments that may be
 +under test before displaying the combined coverage.
++
++
  Adding tests
  ~~~~~~~~~~~~
  Tests are found in the tests subdirectory, inside of which

buildstream/_frontend/cli.py

@@ -560,7 +560,7 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command)
      else:
          scope = Scope.RUN
 -    use_buildtree = False
 +    use_buildtree = None
      with app.initialized():
          if not element:
@@ -568,7 +568,8 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command)
              if not element:
                  raise AppError('Missing argument "ELEMENT".')
 -        dependencies = app.stream.load_selection((element,), selection=PipelineSelection.NONE)
 +        dependencies = app.stream.load_selection((element,), selection=PipelineSelection.NONE,
 +                                                 use_artifact_config=True)
          element = dependencies[0]
          prompt = app.shell_prompt(element)
          mounts = [
@@ -577,18 +578,19 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command)
+         ]
          cached = element._cached_buildtree()
 -        if cli_buildtree == "always":
 -            if cached:
 -                use_buildtree = True
 -            else:
 -                raise AppError("No buildtree is cached but the use buildtree option was specified")
 -        elif cli_buildtree == "never":
 -            pass
 -        elif cli_buildtree == "try":
 -            use_buildtree = cached
 +        if cli_buildtree in ("always", "try"):
 +            use_buildtree = cli_buildtree
 +            if not cached and use_buildtree == "always":
 +                click.echo("Warning: buildtree is not cached locally, will attempt to pull from available remotes")
          else:
 -            if app.interactive and cached:
 -                use_buildtree = bool(click.confirm('Do you want to use the cached buildtree?'))
 +            # If the value has defaulted to ask and in non interactive mode, don't consider the buildtree, this
 +            # being the default behaviour of the command
 +            if app.interactive and cli_buildtree == "ask":
 +                if cached and bool(click.confirm('Do you want to use the cached buildtree?')):
 +                    use_buildtree = "always"
 +                elif not cached and bool(click.confirm('Do you want to attempt to pull the cached buildtree?')):
 +                    use_buildtree = "try"
++
          if use_buildtree and not element._cached_success():
              click.echo("Warning: using a buildtree from a failed build.")

buildstream/_stream.py

@@ -100,16 +100,19 @@ class Stream():
      #    targets (list of str): Targets to pull
      #    selection (PipelineSelection): The selection mode for the specified targets
      #    except_targets (list of str): Specified targets to except from fetching
 +    #    use_artifact_config (bool): If artifact remote config should be loaded
+     #
      # Returns:
      #    (list of Element): The selected elements
      def load_selection(self, targets, *,
                         selection=PipelineSelection.NONE,
 -                       except_targets=()):
 +                       except_targets=(),
 +                       use_artifact_config=False):
          elements, _ = self._load(targets, (),
                                   selection=selection,
                                   except_targets=except_targets,
 -                                 fetch_subprojects=False)
 +                                 fetch_subprojects=False,
 +                                 use_artifact_config=use_artifact_config)
          return elements
      # shell()
@@ -124,7 +127,7 @@ class Stream():
      #    mounts (list of HostMount): Additional directories to mount into the sandbox
      #    isolate (bool): Whether to isolate the environment like we do in builds
      #    command (list): An argv to launch in the sandbox, or None
 -    #    usebuildtree (bool): Wheather to use a buildtree as the source.
 +    #    usebuildtree (str): Whether to use a buildtree as the source, given cli option
+     #
      # Returns:
      #    (int): The exit code of the launched shell
@@ -134,7 +137,7 @@ class Stream():
                mounts=None,
                isolate=False,
                command=None,
 -              usebuildtree=False):
 +              usebuildtree=None):
          # Assert we have everything we need built, unless the directory is specified
          # in which case we just blindly trust the directory, using the element
@@ -149,8 +152,30 @@ class Stream():
                  raise StreamError("Elements need to be built or downloaded before staging a shell environment",
                                    detail="\n".join(missing_deps))
 +        # Check if we require a pull queue attempt, with given artifact state and context
 +        if usebuildtree:
 +            if not element._cached_buildtree():
 +                buildtree = False
 +                require_buildtree = self._buildtree_pull_required([element])
 +                # Attempt a pull queue for the given element if remote and context allow it
 +                if require_buildtree:
 +                    self._message(MessageType.INFO, "Attempting to fetch missing artifact buildtree")
 +                    self._add_queue(PullQueue(self._scheduler))
 +                    self._enqueue_plan(require_buildtree)
 +                    self._run()
 +                    # Now check if the buildtree was successfully fetched
 +                    if element._cached_buildtree():
 +                        buildtree = True
 +                if not buildtree:
 +                    if usebuildtree == "always":
 +                        raise StreamError("Buildtree is not cached locally or in available remotes")
 +                    else:
 +                        self._message(MessageType.INFO, """Buildtree is not cached locally or in available remotes,
 +                                                        shell will be loaded without it""")
 +                        usebuildtree = False
++
          return element._shell(scope, directory, mounts=mounts, isolate=isolate, prompt=prompt, command=command,
 -                              usebuildtree=usebuildtree)
 +                              usebuildtree=bool(usebuildtree))
      # build()
+     #

tests/integration/build-tree.py

@@ -140,7 +140,7 @@ def test_buildtree_pulled(cli, tmpdir, datafiles):
          res.assert_success()
 -# This test checks for correct behaviour if a buildtree is not present.
 +# This test checks for correct behaviour if a buildtree is not present in the local cache.
  @pytest.mark.datafiles(DATA_DIR)
  @pytest.mark.skipif(IS_LINUX and not HAVE_BWRAP, reason='Only available with bubblewrap on Linux')
  def test_buildtree_options(cli, tmpdir, datafiles):
@@ -155,6 +155,7 @@ def test_buildtree_options(cli, tmpdir, datafiles):
          result = cli.run(project=project, args=['build', element_name])
          result.assert_success()
          assert cli.get_element_state(project, element_name) == 'cached'
 +        assert share.has_artifact('test', element_name, cli.get_element_key(project, element_name))
          # Discard the cache
          cli.configure({
@@ -167,8 +168,6 @@ def test_buildtree_options(cli, tmpdir, datafiles):
          result = cli.run(project=project, args=['pull', '--deps', 'all', element_name])
          result.assert_success()
 -        # The above is the simplest way I know to create a local cache without any buildtrees.
+-
          # Check it's not using the cached build tree
          res = cli.run(project=project, args=[
              'shell', '--build', element_name, '--use-buildtree', 'never', '--', 'cat', 'test'
@@ -176,24 +175,51 @@ def test_buildtree_options(cli, tmpdir, datafiles):
          res.assert_shell_error()
          assert 'Hi' not in res.output
 -        # Check it's not correctly handling the lack of buildtree
 +        # Check it's not using the cached build tree, default is to ask, and fall back to not
 +        # for non interactive behavior
          res = cli.run(project=project, args=[
 -            'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
 +            'shell', '--build', element_name, '--', 'cat', 'test'
          ])
          res.assert_shell_error()
          assert 'Hi' not in res.output
 -        # Check it's not using the cached build tree, default is to ask, and fall back to not
 -        # for non interactive behavior
 +        # Check correctly handling the lack of buildtree, with 'try' not attempting to
 +        # pull the buildtree as the user context is by default set to not pull them
          res = cli.run(project=project, args=[
 -            'shell', '--build', element_name, '--', 'cat', 'test'
 +            'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
          ])
 -        res.assert_shell_error()
          assert 'Hi' not in res.output
 +        assert 'Attempting to fetch missing artifact buildtrees' not in res.stderr
 +        assert """Buildtree is not cached locally or in available remotes,
 +                shell will be loaded without it"""
 -        # Check it's using the cached build tree
 +        # Check correctly handling the lack of buildtree, with 'try' attempting and succeeding
 +        # to pull the buildtree as the user context allow the pulling of buildtrees and it is
 +        # available in the remote
 +        res = cli.run(project=project, args=[
 +            '--pull-buildtrees', 'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
 +        ])
 +        assert 'Attempting to fetch missing artifact buildtree' in res.stderr
 +        assert 'Hi' in res.output
 +        shutil.rmtree(os.path.join(os.path.join(cli.directory, 'artifacts2')))
 +        assert cli.get_element_state(project, element_name) != 'cached'
++
 +        # Check it's not loading the shell at all with always set for the buildtree, when the
 +        # user context does not allow for buildtree pulling
 +        result = cli.run(project=project, args=['pull', '--deps', 'all', element_name])
 +        result.assert_success()
          res = cli.run(project=project, args=[
              'shell', '--build', element_name, '--use-buildtree', 'always', '--', 'cat', 'test'
          ])
          res.assert_main_error(ErrorDomain.PROG_NOT_FOUND, None)
 +        assert 'Buildtree is not cached locally or in available remotes' in res.stderr
          assert 'Hi' not in res.output
 +        assert 'Attempting to fetch missing artifact buildtree' not in res.stderr
++
 +        # Check that when user context is set to pull buildtrees and a remote has the buildtree,
 +        # 'always' will attempt and succeed at pulling the missing buildtree.
 +        res = cli.run(project=project, args=[
 +            '--pull-buildtrees', 'shell', '--build', element_name, '--use-buildtree', 'always', '--', 'cat', 'test'
 +        ])
 +        assert 'Hi' in res.output
 +        assert 'Attempting to fetch missing artifact buildtree' in res.stderr

tox.ini

 +#
 +# Tox global configuration
 +#
  [tox]
  envlist = py35,py36,py37
  skip_missing_interpreters = true
 +#
 +# Defaults for all environments
 +#
 +# Anything specified here is iherited by the sections
 +#
  [testenv]
 -commands = pytest {posargs}
 +commands =
 +    pytest --basetemp {envtmpdir} {posargs}
 +    mkdir -p .coverage-reports
 +    mv {envtmpdir}/.coverage {toxinidir}/.coverage-reports/.coverage.{env:COVERAGE_PREFIX:}{envname}
  deps =
      -rrequirements/requirements.txt
      -rrequirements/dev-requirements.txt
@@ -13,6 +24,32 @@ passenv =
      GI_TYPELIB_PATH
      INTEGRATION_CACHE
 +#
 +# These keys are not inherited by any other sections
 +#
 +setenv =
 +    py{35,36,37}: COVERAGE_FILE = {envtmpdir}/.coverage
 +whitelist_externals =
 +    py{35,36,37}:
 +        mv
 +        mkdir
++
 +#
 +# Coverage reporting
 +#
 +[testenv:coverage]
 +commands =
 +    - coverage combine --rcfile={toxinidir}/.coveragerc {toxinidir}/.coverage-reports/
 +    coverage report --rcfile={toxinidir}/.coveragerc -m
 +deps =
 +    -rrequirements/requirements.txt
 +    -rrequirements/dev-requirements.txt
 +setenv =
 +    COVERAGE_FILE = {toxinidir}/.coverage-reports/.coverage
++
 +#
 +# Running linters
 +#
  [testenv:lint]
  commands =
      pycodestyle
@@ -22,6 +59,9 @@ deps =
      -rrequirements/dev-requirements.txt
      -rrequirements/plugin-requirements.txt
 +#
 +# Building documentation
 +#
  [testenv:docs]
  commands =
      make -C doc

[Notes] [Git][BuildStream/buildstream][tpollard/829] 6 commits: Support running test environments in parallel with `detox`

Tom Pollard pushed to branch tpollard/829 at BuildStream / buildstream

Commits:

8 changed files:

Changes: