Skip to content

Conversation

@Szelethus
Copy link
Contributor

@Szelethus Szelethus commented Mar 31, 2025

This PR adds some tests that utilize the rules found in
https://github.com/Ericsson/codechecker_bazel. The tests are added under
the analyzer directory, as these are plain analysis runs, only invoked
through bazel.

These are the things to highlight:

  • codechecker_bazel only supports bazel 6. If you have seen/heard of a
    bazel feature you are missing form this commit, its likely missing
    from bazel 6 is well.

  • Bazel creates a lot of temporary files and directories, all of these
    need to ignored from .gitignore and pytest.

  • Bazel wipes the environment. This means that for the eventual
    CodeChecker call, the proper environment has to be reassembled again.
    This is mostly done via the bazel rules, but I had a lot of trouble
    with the wrapper script finding CodeChecker in the tests. Becuase of
    this, I manually prepend the CodeChecker bin folder onto PATH.

  • The actual tests (which were copy-pasted from the codechecker_bazel
    repository) were made disregarding gcc, cppcheck and infer. These
    things were adjusted in the BUILD file (only specific analyzers were
    enabled).

  • The lock file is usually not ignored. Not too sure on that.

  • The WORKSPACE file is likely among the most interesting -- this is how
    you can use the codechecker bazel rules in your own codebase. Note how
    we don't clone the main branch of the codechecker_bazel repository for
    now, but I already have plenty of PRs about to be merged in that
    repository as well, so this will change back to the main branch soon.

  • In the init.py file I tried to prepend CodeChecker to PATH, but it
    never worked out.

  • test_bazel_test_clang_ctu_fail is still failing. Seems like CTU is not
    working yet. For the time being (because this PR has been stagnating
    for long enough) I just commented it out.

  • The patch depends on [bugfix] Pass the correct interpreter from bin/CodeChecker to the analyzers #4558.

@Szelethus Szelethus added test ☑️ Adding or refactoring tests analyzer 📈 Related to the analyze commands (analysis driver) bazel 🧩 Directly or inderectly related to bazel support labels Mar 31, 2025
@Szelethus Szelethus requested a review from dkrupp March 31, 2025 15:48
@Szelethus Szelethus added the WIP 💣 Work In Progress label Apr 1, 2025
@Szelethus Szelethus force-pushed the bazel_subrepo branch 2 times, most recently from 65b2319 to c821ee8 Compare May 12, 2025 14:47
@Szelethus Szelethus removed the WIP 💣 Work In Progress label May 12, 2025
@Szelethus Szelethus changed the title [WIP] Add tests for Bazel [bazel] Add tests from codechecker_bazel to the analyzer repository May 12, 2025
@Szelethus Szelethus added this to the release 6.27.0 milestone May 12, 2025
This PR adds some tests that utilize the rules found in
https://github.com/Ericsson/codechecker_bazel. The tests are added under
the analyzer directory, as these are plain analysis runs, only invoked
through bazel.

These are the things to highlight:
* codechecker_bazel only supports bazel 6. If you have seen/heard of a
  bazel feature you are missing form this commit, its likely missing
  from bazel 6 is well.

* Bazel creates a lot of temporary files and directories, all of these
  need to ignored from .gitignore and pytest.

* Bazel wipes the environment. This means that for the eventual
  CodeChecker call, the proper environment has to be reassembled again.
  This is mostly done via the bazel rules, but I had a lot of trouble
  with the wrapper script finding CodeChecker in the tests. Becuase of
  this, I manually prepend the CodeChecker bin folder onto PATH.

* The actual tests (which were copy-pasted from the codechecker_bazel
  repository) were made disregarding gcc, cppcheck and infer. These
  things were adjusted in the BUILD file (only specific analyzers were
  enabled).

* The lock file is usually not ignored. Not too sure on that.

* The WORKSPACE file is likely among the most interesting -- this is how
  you can use the codechecker bazel rules in your own codebase. Note how
  we don't clone the main branch of the codechecker_bazel repository for
  now, but I already have plenty of PRs about to be merged in that
  repository as well, so this will change back to the main branch soon.

* In the __init__.py file I tried to prepend CodeChecker to PATH, but it
  never worked out.

* test_bazel_test_clang_ctu_fail is still failing. Seems like CTU is not
  working yet. For the time being (because this PR has been stagnating
  for long enough) I just commented it out.

* The patch depends on Ericsson#4558.
Copy link
Member

@dkrupp dkrupp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's really great that you made this test working! Congrats for that!

I see that these tests are coming from the https://github.com/Ericsson/codechecker_bazel/
repo.

These tests are testing the bazel rules laid out in the bzl files in that repo: codechecker_config, codechecker, codechecker_test, code_checker_test, clang_tidy_test etc.

I think a much more natural place for this github action would be in the codechecker_bazel repo, beacause they are essentially testing the implementation of those rules.

It would look odd, to need to modify these tests in the codechecker repo, when an action is renamed or rewritten in the other bazel repo.

The codechekcer bazel repo has CodeChecker as its dependency and not the other way around.

I understand the motivation for having a regression test in codechecker as codechecker changes more frequently, but still I would not duplicate test code this way.

Since codechecker command line is fairly stable (or backward compatible), I would think these tests would not break frequently.

Vscode plugin (which also depends on CodeChecker command line IF) tests are also not placed in the CodeChecker repo, rather in the vscode plugin repo.

So I think we should follow a similar login there.

What do you think?

@Szelethus
Copy link
Contributor Author

Moved tests here.
Ericsson/codechecker_bazel#5

@Szelethus Szelethus marked this pull request as draft May 19, 2025 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

analyzer 📈 Related to the analyze commands (analysis driver) bazel 🧩 Directly or inderectly related to bazel support test ☑️ Adding or refactoring tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants