From c36a4d12aaf045cd20071bfa6666d63107535bf9 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Mon, 4 Sep 2023 22:25:42 -0700 Subject: [PATCH 1/7] tests: Replace print with testlib.log for PARSEC warn Using just a print was causing this warning to print even with the `-q` flag was passed. The `-q` flag sets the output to machine readable, which the warning statement is not. Change-Id: I139e2565dbc53aaee9027c0e003d34ba800a7ef4 --- .../test_gem5_library_examples.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/gem5/gem5_library_example_tests/test_gem5_library_examples.py b/tests/gem5/gem5_library_example_tests/test_gem5_library_examples.py index b88862fc0d..0882fb0dfd 100644 --- a/tests/gem5/gem5_library_example_tests/test_gem5_library_examples.py +++ b/tests/gem5/gem5_library_example_tests/test_gem5_library_examples.py @@ -29,6 +29,7 @@ This runs simple tests to ensure the examples in `configs/example/gem5_library` still function. They simply check the simulation completed. """ from testlib import * +from testlib.log import * import re import os @@ -171,10 +172,11 @@ gem5_verify_config( length=constants.long_tag, ) -print( - "WARNING: PARSEC tests are disabled. This is due to our GitHub " +log.test_log.message( + "PARSEC tests are disabled. This is due to our GitHub " "Actions self-hosted runners only having 60GB of disk space. The " - "PARSEC Disk image is too big to use." + "PARSEC Disk image is too big to use.", + level=LogLevel.Warn, ) # 'False' is used to disable the tests. if False: # os.access("/dev/kvm", mode=os.R_OK | os.W_OK): From 43226004a108fa60bd858a1c6977976a358a4f12 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Mon, 4 Sep 2023 15:42:35 -0700 Subject: [PATCH 2/7] ext,tests: Fix `--figures` flag when using `./main.py list` Now the "tests/main.py" script will accept the `--fixtures` flag when using the `list` command. This will only list the fixtures needed. To have this implemented `__str__` for the `Fixture` class has been implemented. Change-Id: I4bba26e923c8b0001163726637f2e48c801e92b1 --- ext/testlib/fixture.py | 4 ++++ ext/testlib/main.py | 2 ++ ext/testlib/query.py | 7 +++++++ 3 files changed, 13 insertions(+) diff --git a/ext/testlib/fixture.py b/ext/testlib/fixture.py index 148229a597..0138f018bb 100644 --- a/ext/testlib/fixture.py +++ b/ext/testlib/fixture.py @@ -27,6 +27,7 @@ # Authors: Sean Wilson import testlib.helper as helper +from testlib.configuration import constants class SkipException(Exception): @@ -79,6 +80,9 @@ class Fixture(object): def teardown(self, testitem): pass + def __str__(self): + return f"{self.name} fixture" + def set_global(self): self._is_global = True diff --git a/ext/testlib/main.py b/ext/testlib/main.py index 3888a1ec6b..59ce050bfd 100644 --- a/ext/testlib/main.py +++ b/ext/testlib/main.py @@ -253,6 +253,8 @@ def do_list(): qrunner.list_tests() elif configuration.config.all_tags: qrunner.list_tags() + elif configuration.config.fixtures: + qrunner.list_fixtures() else: qrunner.list_suites() qrunner.list_tests() diff --git a/ext/testlib/query.py b/ext/testlib/query.py index 7b69d7d76b..74541002cd 100644 --- a/ext/testlib/query.py +++ b/ext/testlib/query.py @@ -55,6 +55,13 @@ class QueryRunner(object): for test in suite: log.test_log.message(test.uid, machine_readable=True) + def list_fixtures(self): + log.test_log.message(terminal.separator()) + log.test_log.message('Listing all Test Fixtures.', bold=True) + log.test_log.message(terminal.separator()) + for fixture in self.schedule.all_fixtures(): + log.test_log.message(fixture, machine_readable=True) + def list_suites(self): log.test_log.message(terminal.separator()) log.test_log.message("Listing all Test Suites.", bold=True) From 13b77b3e4173a24ae8ad5dac53d7ae675daf8395 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Mon, 4 Sep 2023 15:46:07 -0700 Subject: [PATCH 3/7] ext,tests: Allow passing of `--uid` to `./main.py list` This is useful for listing the fixtures of a Suite. Change-Id: Id2f1294cc7dea03a6b26e8abc5083886fe0299d9 --- ext/testlib/configuration.py | 7 +++++++ ext/testlib/main.py | 20 +++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/ext/testlib/configuration.py b/ext/testlib/configuration.py index 600f0e13cc..918a3f92b3 100644 --- a/ext/testlib/configuration.py +++ b/ext/testlib/configuration.py @@ -751,6 +751,13 @@ class ListParser(ArgParser): default=False, help="Quiet output (machine readable).", ).add_to(parser) + Argument( + '--uid', + action='store', + default=None, + help='UID of a specific test item to list.' + ).add_to(parser) + common_args.directories.add_to(parser) common_args.bin_path.add_to(parser) diff --git a/ext/testlib/main.py b/ext/testlib/main.py index 59ce050bfd..e9dd892947 100644 --- a/ext/testlib/main.py +++ b/ext/testlib/main.py @@ -242,7 +242,25 @@ def do_list(): entry_message() - test_schedule = load_tests().schedule + if configuration.config.uid: + uid_ = uid.UID.from_uid(configuration.config.uid) + if isinstance(uid_, uid.TestUID): + log.test_log.error('Unable to list a standalone test.\n' + 'Gem5 expects test suites to be the smallest unit ' + ' of test.\n\n' + 'Pass a SuiteUID instead.') + return + test_schedule = loader_mod.Loader().load_schedule_for_suites(uid_) + if get_config_tags(): + log.test_log.warn( + "The '--uid' flag was supplied," + " '--include-tags' and '--exclude-tags' will be ignored." + ) + else: + test_schedule = load_tests().schedule + # Filter tests based on tags + filter_with_config_tags(test_schedule) + filter_with_config_tags(test_schedule) qrunner = query.QueryRunner(test_schedule) From 0337613afcf9803eaa977d787c6d5c03f743db4d Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Mon, 4 Sep 2023 23:34:39 -0700 Subject: [PATCH 4/7] ext,tests: Add `--build-targets` option to `./main.py list` This allows for build target information (i.e., the gem5 binary to be built for the tests) to be returned. Change-Id: I6638b54cbb1822555f58e74938d36043c11108ba --- ext/testlib/configuration.py | 13 +++++++++---- ext/testlib/fixture.py | 14 ++++++++++++++ ext/testlib/main.py | 13 +++++++++---- ext/testlib/query.py | 14 +++++++++++++- tests/gem5/fixture.py | 12 ++++++++++++ 5 files changed, 57 insertions(+), 9 deletions(-) diff --git a/ext/testlib/configuration.py b/ext/testlib/configuration.py index 918a3f92b3..07f636b03b 100644 --- a/ext/testlib/configuration.py +++ b/ext/testlib/configuration.py @@ -744,6 +744,12 @@ class ListParser(ArgParser): default=False, help="List all tags.", ).add_to(parser) + Argument( + "--build-targets", + action="store_true", + default=False, + help="List all the gem5 build targets.", + ).add_to(parser) Argument( "-q", dest="quiet", @@ -752,12 +758,11 @@ class ListParser(ArgParser): help="Quiet output (machine readable).", ).add_to(parser) Argument( - '--uid', - action='store', + "--uid", + action="store", default=None, - help='UID of a specific test item to list.' + help="UID of a specific test item to list.", ).add_to(parser) - common_args.directories.add_to(parser) common_args.bin_path.add_to(parser) diff --git a/ext/testlib/fixture.py b/ext/testlib/fixture.py index 0138f018bb..aa735cc1c5 100644 --- a/ext/testlib/fixture.py +++ b/ext/testlib/fixture.py @@ -29,6 +29,8 @@ import testlib.helper as helper from testlib.configuration import constants +from typing import Optional + class SkipException(Exception): def __init__(self, fixture, testitem): @@ -80,6 +82,18 @@ class Fixture(object): def teardown(self, testitem): pass + def get_get_build_info(self) -> Optional[dict]: + # If this is a gem5 build it will return the target gem5 build path + # and any additional build information. E.g.: + # + # /path/to/gem5/build/NULL/gem5.opt--default=NULL PROTOCOL=MI_example + # + # In this example this may be passed to scons to build gem5 in + # accordance to the test's build requirements. + # + # If this fixtures is not a build of gem5, None is returned. + return None + def __str__(self): return f"{self.name} fixture" diff --git a/ext/testlib/main.py b/ext/testlib/main.py index e9dd892947..f8d04cec56 100644 --- a/ext/testlib/main.py +++ b/ext/testlib/main.py @@ -245,10 +245,12 @@ def do_list(): if configuration.config.uid: uid_ = uid.UID.from_uid(configuration.config.uid) if isinstance(uid_, uid.TestUID): - log.test_log.error('Unable to list a standalone test.\n' - 'Gem5 expects test suites to be the smallest unit ' - ' of test.\n\n' - 'Pass a SuiteUID instead.') + log.test_log.error( + "Unable to list a standalone test.\n" + "Gem5 expects test suites to be the smallest unit " + " of test.\n\n" + "Pass a SuiteUID instead." + ) return test_schedule = loader_mod.Loader().load_schedule_for_suites(uid_) if get_config_tags(): @@ -273,10 +275,13 @@ def do_list(): qrunner.list_tags() elif configuration.config.fixtures: qrunner.list_fixtures() + elif configuration.config.build_targets: + qrunner.list_build_targets() else: qrunner.list_suites() qrunner.list_tests() qrunner.list_tags() + qrunner.list_build_targets() return 0 diff --git a/ext/testlib/query.py b/ext/testlib/query.py index 74541002cd..be1c5a6792 100644 --- a/ext/testlib/query.py +++ b/ext/testlib/query.py @@ -57,11 +57,23 @@ class QueryRunner(object): def list_fixtures(self): log.test_log.message(terminal.separator()) - log.test_log.message('Listing all Test Fixtures.', bold=True) + log.test_log.message("Listing all Test Fixtures.", bold=True) log.test_log.message(terminal.separator()) for fixture in self.schedule.all_fixtures(): log.test_log.message(fixture, machine_readable=True) + def list_build_targets(self): + log.test_log.message(terminal.separator()) + log.test_log.message("Listing all gem5 Build Targets.", bold=True) + log.test_log.message(terminal.separator()) + builds = [] + for fixture in self.schedule.all_fixtures(): + build = fixture.get_get_build_info() + if build and build not in builds: + builds.append(build) + for build in builds: + log.test_log.message(build, machine_readable=True) + def list_suites(self): log.test_log.message(terminal.separator()) log.test_log.message("Listing all Test Suites.", bold=True) diff --git a/tests/gem5/fixture.py b/tests/gem5/fixture.py index d3312c9a63..0f53539b40 100644 --- a/tests/gem5/fixture.py +++ b/tests/gem5/fixture.py @@ -53,6 +53,8 @@ from testlib.helper import log_call, cacheresult, joinpath, absdirpath import testlib.log as log from testlib.state import Result +from typing import Optional, List + class VariableFixture(Fixture): def __init__(self, value=None, name=None): @@ -147,6 +149,10 @@ class SConsFixture(UniqueFixture): obj = super(SConsFixture, cls).__new__(cls, target) return obj + def _setup(self, testitem): + if config.skip_build: + return + def _setup(self, testitem): if config.skip_build: return @@ -204,6 +210,12 @@ class Gem5Fixture(SConsFixture): self.options = ["--default=" + isa.upper(), "PROTOCOL=" + protocol] self.set_global() + def get_get_build_info(self) -> Optional[str]: + build_target = self.target + if self.options: + build_target += " ".join(self.options) + return build_target + class MakeFixture(Fixture): def __init__(self, directory, *args, **kwargs): From efd58f9b723c788200838e9004aa1d652ddddf38 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Fri, 8 Sep 2023 10:17:36 -0700 Subject: [PATCH 5/7] tests: Remove ":" from testing results output dir name Colons in path names is not advisable. Change-Id: I7748a36cabafde69759f7a9892f7b8910470b85e --- tests/gem5/fixture.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/gem5/fixture.py b/tests/gem5/fixture.py index 0f53539b40..05b599dd80 100644 --- a/tests/gem5/fixture.py +++ b/tests/gem5/fixture.py @@ -76,7 +76,7 @@ class TempdirFixture(Fixture): suiteUID = testitem.metadata.uid.suite testUID = testitem.metadata.name testing_result_folder = os.path.join( - config.result_path, "SuiteUID:" + suiteUID, "TestUID:" + testUID + config.result_path, "SuiteUID-" + suiteUID, "TestUID-" + testUID ) # Copy the output files of the run from /tmp to testing-results From 6921d943735905ec1f6b7cc3216d957d7e26c755 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Mon, 11 Sep 2023 15:25:16 -0700 Subject: [PATCH 6/7] python: Recursively create checkpoint dir While there was code present in "serialize.cc" to create the checkpoint directory, it did not do recursively. This patch ensures all the directories are created in a path to the checkpoint directory. Change-Id: Ibcf7f800358fd89946f550b8cfb0cef8b51fceac --- src/python/m5/simulate.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/python/m5/simulate.py b/src/python/m5/simulate.py index 70ed11a9b6..19d5604568 100644 --- a/src/python/m5/simulate.py +++ b/src/python/m5/simulate.py @@ -318,6 +318,10 @@ def checkpoint(dir): drain() memWriteback(root) + + # Recursively create the checkpoint directory if it does not exist. + os.makedirs(dir, exist_ok=True) + print("Writing checkpoint") _m5.core.serializeAll(dir) From 561f3bd75b7bb940515c0e7acd8d46f4d2f0a4d7 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Sun, 3 Sep 2023 23:33:59 -0700 Subject: [PATCH 7/7] misc,tests: Split testlib CI Tests to one dir-per-job This splits the CI Tests to one job per sub-directory in "tests/gem5" via a matrix. Advantages: * We can utilize more runners to run the quick tests. This should mean tests run quicker. * This approach does not require editing of the workflow as more tests are added or taken away. * There is now an output artifact for each directory in "tests/gem5" instead of one for the entriety of every quick test in "tests". In addition: * The artifact retention for the test outputs has been increased to 30 days. * The output test artifacts have been renamed to be more descriptive of the job, run, attempt, directory run, and the status. * The 'tar' step has been removed. GitHub's 'action/artifact' can handle directories. Change-Id: I5b3132b424e3769d81d9cd75db2a8c59dbe4a7e5 --- .github/workflows/ci-tests.yaml | 140 +++++++++++++++++++++++--------- 1 file changed, 101 insertions(+), 39 deletions(-) diff --git a/.github/workflows/ci-tests.yaml b/.github/workflows/ci-tests.yaml index 656f41f4bc..5a92422e79 100644 --- a/.github/workflows/ci-tests.yaml +++ b/.github/workflows/ci-tests.yaml @@ -46,27 +46,6 @@ jobs: "curl -Lo $f https://gerrit-review.googlesource.com/tools/hooks/commit-msg ; chmod +x $f\n Then amend the commit with git commit --amend --no-edit, and update your pull request." exit 1 - build-gem5: - runs-on: [self-hosted, linux, x64, build] - if: github.event.pull_request.draft == false - container: ghcr.io/gem5/ubuntu-22.04_all-dependencies:latest - needs: [pre-commit, check-for-change-id] # only runs if pre-commit and change-id passes - outputs: - artifactname: ${{ steps.name.outputs.test }} - steps: - - uses: actions/checkout@v3 - - id: name - run: echo "test=$(date +"%Y-%m-%d_%H.%M.%S")-artifact" >> $GITHUB_OUTPUT - - - name: Build gem5 - run: | - scons build/ALL/gem5.opt -j $(nproc) - - uses: actions/upload-artifact@v3 - with: - name: ${{ steps.name.outputs.test }} - path: build/ALL/gem5.opt - - run: echo "This job's status is ${{ job.status }}." - unittests-all-opt: runs-on: [self-hosted, linux, x64, run] if: github.event.pull_request.draft == false @@ -80,39 +59,122 @@ jobs: run: scons build/ALL/unittests.opt -j $(nproc) - run: echo "This job's status is ${{ job.status }}." - testlib-quick: + testlib-quick-matrix: + runs-on: [self-hosted, linux, x64, run] + if: github.event.pull_request.draft == false + # In order to make sure the environment is exactly the same, we run in + # the same container we use to build gem5 and run the testlib tests. This + container: ghcr.io/gem5/ubuntu-22.04_all-dependencies:latest + needs: [pre-commit, check-for-change-id] + steps: + - uses: actions/checkout@v3 + + # Unfortunately the 'ubunutu-latest' image doesn't have jq installed. + # We therefore need to install it as a step here. + - name: Install jq + run: apt install -y jq + + - name: Get directories for testlib-quick + working-directory: "${{ github.workspace }}/tests" + id: dir-matrix + run: echo "test-dirs-matrix=$(find gem5/* -type d -maxdepth 0 | jq -ncR '[inputs]')" >>$GITHUB_OUTPUT + + - name: Get the build targets for testlib-quick-gem5-builds + working-directory: "${{ github.workspace }}/tests" + id: build-matrix + run: echo "build-matrix=$(./main.py list --build-targets -q | jq -ncR '[inputs]')" >>$GITHUB_OUTPUT + + outputs: + build-matrix: ${{ steps.build-matrix.outputs.build-matrix }} + test-dirs-matrix: ${{ steps.dir-matrix.outputs.test-dirs-matrix }} + + testlib-quick-gem5-builds: + runs-on: [self-hosted, linux, x64, build] + if: github.event.pull_request.draft == false + container: ghcr.io/gem5/ubuntu-22.04_all-dependencies:latest + needs: [pre-commit, check-for-change-id, testlib-quick-matrix] + strategy: + matrix: + build-target: ${{ fromJson(needs.testlib-quick-matrix.outputs.build-matrix) }} + steps: + - uses: actions/checkout@v3 + - name: Build gem5 + run: scons ${{ matrix.build-target }} -j $(nproc) + + # Upload the gem5 binary as an artifact. + # Note: the "achor.txt" file is a hack to make sure the paths are + # preserverd in the artifact. The upload-artifact action finds the + # closest common directory and uploads everything relative to that. + # E.g., if we upload "build/ARM/gem5.opt" and "build/RISCV/gem5.opt" + # Then upload-artifact will upload "ARM/gem5.opt" and "RISCV/gem5.opt", + # stripping the "build" directory. By adding the "anchor.txt" file, we + # ensure the "build" directory is preserved. + - run: echo "anchor" > anchor.txt + - uses: actions/upload-artifact@v3 + with: + name: ci-tests-${{ github.run_number }}-testlib-quick-all-gem5-builds + path: | + build/*/gem5.* + anchor.txt + retention-days: 7 + + testlib-quick-execution: runs-on: [self-hosted, linux, x64, run] if: github.event.pull_request.draft == false container: ghcr.io/gem5/ubuntu-22.04_all-dependencies:latest - needs: [pre-commit, build-gem5, check-for-change-id] + needs: [pre-commit, check-for-change-id, testlib-quick-matrix, testlib-quick-gem5-builds] timeout-minutes: 360 # 6 hours + strategy: + fail-fast: false + matrix: + test-dir: ${{ fromJson(needs.testlib-quick-matrix.outputs.test-dirs-matrix) }} steps: - name: Clean runner run: rm -rf ./* || true rm -rf ./.??* || true rm -rf ~/.cache || true + + # Checkout the repository then download the gem5.opt artifact. - uses: actions/checkout@v3 - uses: actions/download-artifact@v3 with: - name: ${{needs.build-gem5.outputs.artifactname}} - path: build/ALL - - run: chmod u+x build/ALL/gem5.opt - - name: The TestLib CI Tests - working-directory: ${{ github.workspace }}/tests - run: ./main.py run --skip-build -vv - - name: create zip of results - if: success() || failure() + name: ci-tests-${{ github.run_number }}-testlib-quick-all-gem5-builds + + # Check that the gem5.opt artifact exists and is executable. + - name: Chmod gem5.{opt,debug,fast} to be executable run: | - apt-get -y install zip - zip -r output.zip tests/testing-results - - name: upload zip + find . -name "gem5.opt" -exec chmod u+x {} \; + find . -name "gem5.debug" -exec chmod u+x {} \; + find . -name "gem5.fast" -exec chmod u+x {} \; + + # Run the testlib quick tests in the given directory. + - name: Run "tests/${{ matrix.test-dir }}" TestLib quick tests + id: run-tests + working-directory: ${{ github.workspace }}/tests + run: ./main.py run --skip-build -vv ${{ matrix.test-dir }} + + # Get the basename of the matrix.test-dir path (to name the artifact). + - name: Sanatize test-dir for artifact name + id: sanitize-test-dir + if: success() || failure() + run: echo "sanatized-test-dir=$(echo '${{ matrix.test-dir }}' | sed 's/\//-/g')" >> $GITHUB_OUTPUT + + # Upload the tests/testing-results directory as an artifact. + - name: Upload test results if: success() || failure() uses: actions/upload-artifact@v3 - env: - MY_STEP_VAR: ${{github.job}}_COMMIT.${{github.sha}}_RUN.${{github.run_id}}_ATTEMPT.${{github.run_attempt}} with: - name: ${{ env.MY_STEP_VAR }} - path: output.zip - retention-days: 7 + name: ci-tests-run-${{ github.run_number }}-attempt-${{ github.run_attempt }}-testlib-quick-${{ steps.sanitize-test-dir.outputs.sanatized-test-dir }}-status-${{ steps.run-tests.outcome }}-output + path: tests/testing-results + retention-days: 30 + + testlib-quick: + # It is 'testlib-quick' which needs to pass for the pull request to be + # merged. The 'testlib-quick-execution' is a matrix job which runs all the + # the testlib quick tests. This job is therefore a stub which will pass if + # all the testlib-quick-execution jobs pass. + runs-on: [self-hosted, linux, x64, run] + needs: testlib-quick-execution + steps: - run: echo "This job's status is ${{ job.status }}."