Rewrite testfiles to have multiversion testing locally by furtib · Pull Request #165 · Ericsson/rules_codechecker

furtib · 2026-01-22T12:08:25Z

Why:
We have no automated way to run tests for multiple Bazel versions.

What:

Add test_all_bazel_version.py to change the .bazelversion file's contents between running the tests.
Add usage guide to README.md

Addresses:
none

nettle

Very good initiative @furtib!
But, sorry, test runner is really bad idea.
To move forward, may I suggest splitting this patch to 2:

This one to actually address multiversion testing locally
New one to re-implement tests that they clean after themselves without external cleaners

If I may, the following might help for the second part:

class TemporaryDir:
    """
    Safely creates and destroys temporary folder
    """
    def __init__(self):
        self._temp_dir = None

    def __enter__(self):
        self._temp_dir = tempfile.mkdtemp()
        logging.debug("Created %s", self._temp_dir)
        return self._temp_dir

    def __exit__(self, tipe, value, traceback):
        shutil.rmtree(self._temp_dir)
        logging.debug("Removed %s", self._temp_dir)

nettle · 2026-01-22T20:26:58Z

test/foss/yaml-cpp/init.sh

 # See the License for the specific language governing permissions and
 # limitations under the License.

 git clone --recurse https://github.com/jbeder/yaml-cpp.git test-proj


I was wondering why do we need --recurse?
Does this repo have submodules?

Just stayed in from the original script.

nettle · 2026-01-22T20:28:54Z

test/foss/yaml-cpp/init.sh

+git reset --hard
+git clean -fd


I guess rm -rf test-proj is much safer.
Actually, we must always clean up after test, but that's different discussion

nettle · 2026-01-22T20:30:42Z

test/test_foss.sh

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+python3 test_runner.py foss @a


nettle · 2026-01-22T20:35:40Z

test/test_runner.py

+This file runs tests.
+For unittest pass `unit` as parameter
+For FOSS tests pass `foss` as parameter
+For verbosity use `-vvv`
+To clean up FOSS test artifacts use `--clean`
+To run the specified tests on all supported Bazel versions use `--all`


I'm strictly against any test runners and wrappers around wrappers :)
If we cannot run python unittests via standard python -m unittest, if we need pre-initialization, if tests do not clean themselves, if we need special cleaner, then something is definitely wrong.

furtib · 2026-01-26T16:52:07Z

I reworked it a bit, removed anything that is not strictly necessary for automated testing of multiple Bazel versions.

I may be short-sighted, but I don't know how I could do this without the help of a wrapper around python3 -m unittests discover @a. However, for regular testing, python3 -m unittests discover @a should and does work!

nettle

To me this patch is quite controversial.
Here are my reasons:

I think it does not help much in running tests locally
It does aim only at bazel multiversion, but not other tools (at least CodeChecker)
It implies we always have bazelisk instead of bazel which not the case
It makes tests depend on .github which is really not good
It adds more unnecessary layering which hides testing functionality

nettle · 2026-01-26T20:57:21Z

test/test.sh

@@ -1,2 +1,17 @@
 #!/bin/bash


I think the following shebang is recommended:

#!/usr/bin/env bash

nettle · 2026-01-26T20:58:36Z

test/test.sh

@@ -1,2 +1,17 @@
 #!/bin/bash
-python3 -m unittest discover unit $@


If we introduce test/test_foss.sh then this file should be test/test_unit.sh

nettle · 2026-01-26T20:58:47Z

test/test.sh

+# Copyright 2023 Ericsson AB
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.


I guess this is not much needed for "convenience" scripts

We have the license in empts __init__.py files too.
Better safe than sorry.

nettle · 2026-01-26T21:02:11Z

test/test_all_bazel_versions.py

+This script replaces the content of the .bazelversion file
+from .github/bazel_version.json and runs the equivalent of
+python3 -m unittest discover @a
+For unittest pass `unit` as parameter
+For FOSS tests pass `foss` as parameter #TODO: Add cleanup between FOSS runs
+For verbosity use `-vvv`


To be honest I dont understand the meaning of this so far...
Looks like something which depends on bazelisk. Am I right?

You are right, this does depend on bazelisk. My goal was to make this script as simple as it can get. And to be honest, I didn't think testing multiple Bazel versions without bazelisk was even possible.

furtib · 2026-01-27T15:32:40Z

Now that we seem to be on board with Mise (#132), is this patch still relevant?

nettle · 2026-01-27T22:11:42Z

Now that we seem to be on board with Mise (#132), is this patch still relevant?

Probably not...
I would vote to put this back to draft or cancel

furtib · 2026-01-28T05:51:20Z

Closing this PR since this issue has been addressed in #132.

furtib requested review from Szelethus and nettle January 22, 2026 12:08

furtib self-assigned this Jan 22, 2026

furtib added enhancement New feature or request CI 📦 test ☑️ Adding or refactoring tests labels Jan 22, 2026

furtib force-pushed the refactor-testing branch from 2c48880 to cb78eed Compare January 22, 2026 12:28

furtib mentioned this pull request Jan 22, 2026

Support FOSS tests running locally #149

Draft

nettle requested changes Jan 22, 2026

View reviewed changes

furtib added 3 commits January 26, 2026 17:36

Rewrite testfiles to have multiversion testing locally

45fb680

Make multiversion running optional

ceb91d8

Make multiversion running optional and add a clean option

aedeed9

furtib force-pushed the refactor-testing branch from 9a28309 to e640e3a Compare January 26, 2026 16:39

furtib added 3 commits January 26, 2026 17:43

Add usage guide for test scripts

9fe0b54

Rewrite test logic in python for better readability

fe6f072

Rewrite test_runner to only run tests for multiple bazel versions

c65bfb6

furtib force-pushed the refactor-testing branch from e640e3a to c65bfb6 Compare January 26, 2026 16:45

furtib requested a review from nettle January 26, 2026 16:52

nettle requested changes Jan 26, 2026

View reviewed changes

furtib closed this Jan 28, 2026

furtib mentioned this pull request Jan 28, 2026

Support FOSS tests running locally #148

Open

		@@ -1,2 +1,17 @@
		#!/bin/bash
		python3 -m unittest discover unit $@

Conversation

furtib commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nettle left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

furtib commented Jan 26, 2026

Uh oh!

nettle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

furtib commented Jan 27, 2026

Uh oh!

nettle commented Jan 27, 2026

Uh oh!

furtib commented Jan 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

furtib commented Jan 22, 2026 •

edited

Loading

nettle left a comment •

edited

Loading