Commit 83a76a13 authored by Greg Mierzwinski's avatar Greg Mierzwinski
Browse files

Bug 1832669 - Reset extra_args between pushes, and fail on bad arguments.... 

Bug 1832669 - Reset extra_args between pushes, and fail on bad arguments. r=perftest-reviewers,kshampur

This patch fixes an issue where the extra_args aren't reset between the base, and new runs. It also adds an additional failure when unknown arguments are provided.

Differential Revision: https://phabricator.services.mozilla.com/D177820
parent 9a34e8ff

tools/tryselect/selectors/perf.py

+6 −4
Original line number Diff line number Diff line
@@ -1025,9 +1025,10 @@ class PerfParser(CompareParser): 
            if not (dry_run or single_run or base_revision_treeherder):

                # Setup the base revision, and try config. This lets us change the options

                # we run the tests with through the PERF_FLAGS environment variable.

                base_extra_args = list(extra_args)

                base_try_config = copy.deepcopy(try_config)

                comparator_obj.setup_base_revision(extra_args)

                PerfParser.setup_try_config(base_try_config, extra_args)

                comparator_obj.setup_base_revision(base_extra_args)

                PerfParser.setup_try_config(base_try_config, base_extra_args)



                with redirect_stdout(log_processor):

                    # XXX Figure out if we can use the `again` selector in some way
@@ -1053,10 +1054,11 @@ class PerfParser(CompareParser): 


                comparator_obj.teardown_base_revision()



            comparator_obj.setup_new_revision(extra_args)

            new_extra_args = list(extra_args)

            comparator_obj.setup_new_revision(new_extra_args)

            PerfParser.setup_try_config(

                try_config,

                extra_args,

                new_extra_args,

                base_revision_treeherder=base_revision_treeherder,

            )

tools/tryselect/selectors/perfselector/perfcomparators.py

+6 −0
Original line number Diff line number Diff line
@@ -151,6 +151,12 @@ class BenchmarkComparator(BasePerfComparator): 
            if arg.startswith(arg_prefix):

                _, settings = arg.split(arg_prefix)

                setting, val = settings.split("=")

                if setting not in benchmark_info:

                    raise BadComparatorArgs(

                        f"Unknown argument provided `{setting}`. Only the following "

                        f"are available (prefixed with `{arg_prefix}`): "

                        f"{list(benchmark_info.keys())}"

                    )

                benchmark_info[setting] = val



        # Parse the link for any required information

tools/tryselect/test/test_perfcomparators.py

+15 −0
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@ from unittest import mock 
import mozunit

import pytest

from tryselect.selectors.perfselector.perfcomparators import (

    BadComparatorArgs,

    BenchmarkComparator,

    ComparatorNotFound,

    get_comparator,
@@ -106,6 +107,20 @@ def test_benchmark_comparator_no_pr_links(): 
        _verify_extra_args(extra_args)





def test_benchmark_comparator_bad_args():

    comparator = BenchmarkComparator(

        None,

        None,

        None,

        [

            "base-bad-args=val",

        ],

    )



    with pytest.raises(BadComparatorArgs):

        comparator.setup_base_revision([])





def test_get_comparator_bad_name():

    with pytest.raises(ComparatorNotFound):

        get_comparator("BadName")