[Notes] [Git][BuildStream/buildstream][jennis/add_artifacts

Tristan Van Berkom pushed to branch jennis/add_artifacts_completion at BuildStream / buildstream

Commits:

0b83d024

by Tristan Van Berkom at 2019-01-10T20:02:50Z

_pipeline.py: Fix the planner to retain the original order when depth sorting

The algorithm adds elements to a dictionary and then sorts the dictionary
by the discovered depths while recursing - using an OrderedDict is enough
to ensure a stable order.

This fixes `bst show --deps plan ...` reporting different results on
each invocation.

This fixes an aspect of #824

630e26f1

by Tristan Van Berkom at 2019-01-10T20:03:52Z

testutils/runcli.py: Added Result.get_start_order()

Added a function to report the list of elements which appeared in
a given queue in the order of their first appearance.

d1d7de57

by Tristan Van Berkom at 2019-01-11T04:13:16Z

tests/frontend/order.py: Adding tests ensuring stable element processing order

For each sample project and expected result order, this test ensures that:

  * bst show --deps plan: Always shows the expected build order.

  * bst source fetch: Always executes in the expected build order.

  * bst build: Always builds in the expected order (disabled).

The build part of the test is currently disabled as the scheduler
seems to be messing with the order even in a `--builders 1` scenario.

276b8d48

by Tristan Van Berkom at 2019-01-11T04:42:24Z

Merge branch 'tristan/element-processing-order' into 'master'

Make build plan element list stable

See merge request BuildStream/buildstream!1058

fc3dcec8

by Angelos Evripiotis at 2019-01-11T16:45:00Z

dev-requirements: pytest-cov==2.6.1 for '--no-cov'

Bump the version of pytest-cov, so that we pick up the fix for
the '--no-cov' option breaking in pytest 4.0:

    https://github.com/pytest-dev/pytest-cov/pull/230

This was the error you would get prior to this version:

    _pytest.warning_types.RemovedInPytest4Warning: config.warn has been
    deprecated, use warnings.warn instead

Although it says 'deprecated', it did result in
termination with stack trace.

f86b7ff3

by Angelos Evripiotis at 2019-01-11T16:45:00Z

contributing: '--last-failed' and '--no-cov' tips

To help new folks get to grips with pytest, add some tips on other
frequently used features.

d983f231

by Angelos Evripiotis at 2019-01-11T17:19:06Z

Merge branch 'aevri/tox_no_cov' into 'master'

pytest-cov==2.6.1, contributing: '--last-failed' and '--no-cov' tips

See merge request BuildStream/buildstream!1059

829a2b93

by Tristan Van Berkom at 2019-01-12T21:32:13Z

_scheduler/queues/queue.py: Put elements in the skip list, not jobs

Minor correction, looks like we're not observing this queue
otherwise we'd be seeing crashes.

993e30ae

by Tristan Van Berkom at 2019-01-12T22:02:55Z

Merge branch 'tristan/fix-skipped-elements' into 'master'

_scheduler/queues/queue.py: Put elements in the skip list, not jobs

See merge request BuildStream/buildstream!1063

32732e01

by Javier Jardón at 2019-01-14T09:04:01Z

.gitlab-ci.yml: Use latest freedesktop-sdk 18.08.25

4f5f1184

by Valentin David at 2019-01-14T09:40:56Z

Merge branch 'jjardon/freedesktop-sdk_latest' into 'master'

.gitlab-ci.yml: Use latest freedesktop-sdk 18.08.25

Closes #858

See merge request BuildStream/buildstream!1064

bb80a2b8

by Chandan Singh at 2019-01-14T14:30:15Z

.gitlab-ci.yml: Use newer image for overning aarch tests

During recent reorganizing of `.gitlab-ci.yml` to work with `tox`, seems
like we missed to update the image used by the overnight aarch tests,
meaning that they currently fail due to `tox` being missing from them.

While these tests will be skipped on MRs usually, here is an example of
what the tests look like if they are actually run with this change:
https://gitlab.com/BuildStream/buildstream/-/jobs/145449561.

Fixes #859.

10b3ee62

by Chandan Singh at 2019-01-14T14:57:08Z

Merge branch 'chandan/fix-overnight-aarch' into 'master'

Chandan/fix overnight aarch

Closes #859

See merge request BuildStream/buildstream!1065

542cdaf0

by Jürg Billeter at 2019-01-14T18:46:57Z

_frontend/cli.py: Also check original args for --config in bash completion

The path of the config file generated by testutils for completion tests
is passed as regular argument, not via COMP_WORDS. Use that config file
in complete_artifact() to ensure the test uses the right artifact
directory.

ff666e76

by James Ennis at 2019-01-14T18:46:57Z

completions.py: Add a test for our artifact ref autocompletions

Since the artifact subcommand group has been created, we have been able
to tab complete both element names and artifact refs as arguments to
the artifact subcommands (e.g. bst artifact log), this commit adds a
test for this.

Changes:

.gitlab-ci.yml

@@ -61,7 +61,7 @@ tests-ubuntu-18.04:
    <<: *tests
  overnight-fedora-28-aarch64:
 -  image: buildstream/testsuite-fedora:aarch64-28-06bab030-32a101f6
 +  image: buildstream/testsuite-fedora:aarch64-28-5da27168-32c47d1c
    tags:
      - aarch64
    <<: *tests
@@ -70,6 +70,12 @@ overnight-fedora-28-aarch64:
    except: []
    only:
    - schedules
 +  before_script:
 +    # grpcio needs to be compiled from source on aarch64 so we additionally
 +    # need a C++ compiler here.
 +    # FIXME: Ideally this would be provided by the base image. This will be
 +    # unblocked by https://gitlab.com/BuildStream/buildstream-docker-images/issues/34
 +    - dnf install -y gcc-c++
  tests-unix:
    # Use fedora here, to a) run a test on fedora and b) ensure that we
@@ -141,7 +147,7 @@ docs:
    variables:
      BST_EXT_URL: git+https://gitlab.com/BuildStream/bst-external.git
      BST_EXT_REF: 573843768f4d297f85dc3067465b3c7519a8dcc3 # 0.7.0
 -    FD_SDK_REF: 612f66e218445eee2b1a9d7dd27c9caba571612e # freedesktop-sdk-18.08.19-54-g612f66e2
 +    FD_SDK_REF: freedesktop-sdk-18.08.25-0-g250939d465d6dd7768a215f1fa59c4a3412fc337
    before_script:
    - |
      mkdir -p "${HOME}/.config"

CONTRIBUTING.rst

@@ -1534,6 +1534,10 @@ You can always abort on the first failure by running::
    tox -- -x
 +Similarly, you may also be interested in the ``--last-failed`` and
 +``--failed-first`` options as per the
 +`pytest cache <https://docs.pytest.org/en/latest/cache.html>`_ documentation.
++
  If you want to run a specific test or a group of tests, you
  can specify a prefix to match. E.g. if you want to run all of
  the frontend tests you can do::
@@ -1545,6 +1549,12 @@ If you wanted to run the test_build_track test within frontend/buildtrack.py you
    tox -- tests/frontend/buildtrack.py::test_build_track
 +When running only a few tests, you may find the coverage and timing output
 +excessive, there are options to trim them. Note that coverage step will fail.
 +Here is an example::
++
 +  tox -- --no-cov --durations=1 tests/frontend/buildtrack.py::test_build_track
++
  We also have a set of slow integration tests that are disabled by
  default - you will notice most of them marked with SKIP in the pytest
  output. To run them, you can use::

buildstream/_frontend/cli.py

@@ -2,6 +2,7 @@ import os
  import sys
  from contextlib import ExitStack
  from fnmatch import fnmatch
 +from functools import partial
  from tempfile import TemporaryDirectory
  import click
@@ -111,14 +112,25 @@ def complete_target(args, incomplete):
      return complete_list
 -def complete_artifact(args, incomplete):
 +def complete_artifact(orig_args, args, incomplete):
      from .._context import Context
      ctx = Context()
      config = None
 -    for i, arg in enumerate(args):
 -        if arg in ('-c', '--config'):
 -            config = args[i + 1]
 +    if orig_args:
 +        for i, arg in enumerate(orig_args):
 +            if arg in ('-c', '--config'):
 +                try:
 +                    config = orig_args[i + 1]
 +                except IndexError:
 +                    pass
 +    if args:
 +        for i, arg in enumerate(args):
 +            if arg in ('-c', '--config'):
 +                try:
 +                    config = args[i + 1]
 +                except IndexError:
 +                    pass
      ctx.load(config)
      # element targets are valid artifact names
@@ -128,8 +140,9 @@ def complete_artifact(args, incomplete):
      return complete_list
 -def override_completions(cmd, cmd_param, args, incomplete):
 +def override_completions(orig_args, cmd, cmd_param, args, incomplete):
      """
 +    :param orig_args: original, non-completion args
      :param cmd_param: command definition
      :param args: full list of args typed before the incomplete arg
      :param incomplete: the incomplete text to autocomplete
@@ -150,7 +163,7 @@ def override_completions(cmd, cmd_param, args, incomplete):
                  cmd_param.opts == ['--track-except']):
              return complete_target(args, incomplete)
          if cmd_param.name == 'artifacts':
 -            return complete_artifact(args, incomplete)
 +            return complete_artifact(orig_args, args, incomplete)
      raise CompleteUnhandled()
@@ -161,7 +174,7 @@ def override_main(self, args=None, prog_name=None, complete_var=None,
      # Hook for the Bash completion.  This only activates if the Bash
      # completion is actually enabled, otherwise this is quite a fast
      # noop.
 -    if main_bashcomplete(self, prog_name, override_completions):
 +    if main_bashcomplete(self, prog_name, partial(override_completions, args)):
          # If we're running tests we cant just go calling exit()
          # from the main process.

buildstream/_pipeline.py

@@ -22,6 +22,7 @@
  import os
  import itertools
  from operator import itemgetter
 +from collections import OrderedDict
  from ._exceptions import PipelineError
  from ._message import Message, MessageType
@@ -479,7 +480,7 @@ class Pipeline():
+ #
  class _Planner():
      def __init__(self):
 -        self.depth_map = {}
 +        self.depth_map = OrderedDict()
          self.visiting_elements = set()
      # Here we want to traverse the same element more than once when

buildstream/_scheduler/queues/queue.py

@@ -170,9 +170,9 @@ class Queue():
          skip = [job for job in jobs if self.status(job.element) == QueueStatus.SKIP]
          wait = [job for job in jobs if job not in skip]
 +        self.skipped_elements.extend([job.element for job in skip])
          self._wait_queue.extend(wait)
          self._done_queue.extend(skip)
 -        self.skipped_elements.extend(skip)
      # dequeue()
+     #

requirements/dev-requirements.txt

@@ -2,7 +2,7 @@ coverage==4.4
  pylint==2.2.2
  pycodestyle==2.4.0
  pytest==4.0.2
 -pytest-cov==2.6.0
 +pytest-cov==2.6.1
  pytest-datafiles==2.0
  pytest-env==0.6.2
  pytest-xdist==1.25.0

tests/completions/completions.py

@@ -281,3 +281,44 @@ def test_argument_element_invalid(datafiles, cli, project, cmd, word_idx, expect
  ])
  def test_help_commands(cli, cmd, word_idx, expected):
      assert_completion(cli, cmd, word_idx, expected)
++
++
 +@pytest.mark.datafiles(os.path.join(DATA_DIR, 'project'))
 +def test_argument_artifact(cli, tmpdir, datafiles):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
++
 +    # Build an import element with no dependencies (as there will only be ONE cache key)
 +    result = cli.run(project=project, args=['build', 'import-bin.bst'])  # Has no dependencies
 +    result.assert_success()
++
 +    # Get the key and the artifact ref ($project/$element_name/$key)
 +    key = cli.get_element_key(project, 'import-bin.bst')
 +    artifact = os.path.join('test', 'import-bin', key)
++
 +    # Test autocompletion of the artifact
 +    cmds = [
 +        'bst artifact log ',
 +        'bst artifact log t',
 +        'bst artifact log test/'
 +    ]
++
 +    for i, cmd in enumerate(cmds):
 +        word_idx = 3
 +        result = cli.run(project=project, cwd=project, env={
 +            '_BST_COMPLETION': 'complete',
 +            'COMP_WORDS': cmd,
 +            'COMP_CWORD': str(word_idx)
 +        })
 +        words = []
 +        if result.output:
 +            words = result.output.splitlines()  # This leaves an extra space on each e.g. ['foo.bst ']
 +            words = [word.strip() for word in words]
++
 +            if i == 0:
 +                expected = PROJECT_ELEMENTS + [artifact]  # We should now be able to see the artifact
 +            elif i == 1:
 +                expected = ['target.bst', artifact]
 +            elif i == 2:
 +                expected = [artifact]
++
 +            assert expected == words

tests/frontend/order.py

 +import os
++
 +import pytest
 +from tests.testutils import cli, create_repo
++
 +from buildstream import _yaml
++
 +# Project directory
 +DATA_DIR = os.path.join(
 +    os.path.dirname(os.path.realpath(__file__)),
 +    "project",
 +)
++
++
 +def create_element(repo, name, path, dependencies, ref=None):
 +    element = {
 +        'kind': 'import',
 +        'sources': [
 +            repo.source_config(ref=ref)
 +        ],
 +        'depends': dependencies
 +    }
 +    _yaml.dump(element, os.path.join(path, name))
++
++
 +# This tests a variety of scenarios and checks that the order in
 +# which things are processed remains stable.
 +#
 +# This is especially important in order to ensure that our
 +# depth sorting and optimization of which elements should be
 +# processed first is doing it's job right, and that we are
 +# promoting elements to the build queue as soon as possible
 +#
 +# Parameters:
 +#    targets (target elements): The targets to invoke bst with
 +#    template (dict): The project template dictionary, for create_element()
 +#    expected (list): A list of element names in the expected order
 +#
 +@pytest.mark.datafiles(os.path.join(DATA_DIR))
 +@pytest.mark.parametrize("target,template,expected", [
 +    # First simple test
 +    ('3.bst', {
 +        '0.bst': ['1.bst'],
 +        '1.bst': [],
 +        '2.bst': ['0.bst'],
 +        '3.bst': ['0.bst', '1.bst', '2.bst']
 +    }, ['1.bst', '0.bst', '2.bst', '3.bst']),
++
 +    # A more complicated test with build of build dependencies
 +    ('target.bst', {
 +        'a.bst': [],
 +        'base.bst': [],
 +        'timezones.bst': [],
 +        'middleware.bst': [{'filename': 'base.bst', 'type': 'build'}],
 +        'app.bst': [{'filename': 'middleware.bst', 'type': 'build'}],
 +        'target.bst': ['a.bst', 'base.bst', 'middleware.bst', 'app.bst', 'timezones.bst']
 +    }, ['base.bst', 'middleware.bst', 'a.bst', 'app.bst', 'timezones.bst', 'target.bst']),
 +])
 +@pytest.mark.parametrize("operation", [('show'), ('fetch'), ('build')])
 +def test_order(cli, datafiles, tmpdir, operation, target, template, expected):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    dev_files_path = os.path.join(project, 'files', 'dev-files')
 +    element_path = os.path.join(project, 'elements')
++
 +    # FIXME: Remove this when the test passes reliably.
 +    #
 +    #        There is no reason why the order should not
 +    #        be preserved when the builders is set to 1,
 +    #        the scheduler queue processing still seems to
 +    #        be losing the order.
 +    #
 +    if operation == 'build':
 +        pytest.skip("FIXME: This still only sometimes passes")
++
 +    # Configure to only allow one fetcher at a time, make it easy to
 +    # determine what is being planned in what order.
 +    cli.configure({
 +        'scheduler': {
 +            'fetchers': 1,
 +            'builders': 1
 +        }
 +    })
++
 +    # Build the project from the template, make import elements
 +    # all with the same repo
 +    #
 +    repo = create_repo('git', str(tmpdir))
 +    ref = repo.create(dev_files_path)
 +    for element, dependencies in template.items():
 +        create_element(repo, element, element_path, dependencies, ref=ref)
 +        repo.add_commit()
++
 +    # Run test and collect results
 +    if operation == 'show':
 +        result = cli.run(args=['show', '--deps', 'plan', '--format', '%{name}', target], project=project, silent=True)
 +        result.assert_success()
 +        results = result.output.splitlines()
 +    else:
 +        if operation == 'fetch':
 +            result = cli.run(args=['source', 'fetch', target], project=project, silent=True)
 +        else:
 +            result = cli.run(args=[operation, target], project=project, silent=True)
 +        result.assert_success()
 +        results = result.get_start_order(operation)
++
 +    # Assert the order
 +    print("Expected order: {}".format(expected))
 +    print("Observed result order: {}".format(results))
 +    assert results == expected

tests/testutils/runcli.py

@@ -167,6 +167,23 @@ class Result():
      def assert_shell_error(self, fail_message=''):
          assert self.exit_code == 1, fail_message
 +    # get_start_order()
 +    #
 +    # Gets the list of elements processed in a given queue, in the
 +    # order of their first appearances in the session.
 +    #
 +    # Args:
 +    #    activity (str): The queue activity name (like 'fetch')
 +    #
 +    # Returns:
 +    #    (list): A list of element names in the order which they first appeared in the result
 +    #
 +    def get_start_order(self, activity):
 +        results = re.findall(r'\[\s*{}:(\S+)\s*\]\s*START\s*.*\.log'.format(activity), self.stderr)
 +        if results is None:
 +            return []
 +        return list(results)
++
      # get_tracked_elements()
+     #
      # Produces a list of element names on which tracking occurred

[Notes] [Git][BuildStream/buildstream][jennis/add_artifacts_completion] 15 commits: _pipeline.py: Fix the planner to retain the original order when depth sorting

Tristan Van Berkom pushed to branch jennis/add_artifacts_completion at BuildStream / buildstream

Commits:

9 changed files:

Changes: