Skip to content

LCORE-663: Updated contributing guide #532

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 12, 2025
Merged

LCORE-663: Updated contributing guide #532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@
* [A deliverable is to be considered “done” when](#a-deliverable-is-to-be-considered-done-when)
* [Automation](#automation)
* [Pre-commit hook settings](#pre-commit-hook-settings)
* [Code coverage measurement](#code-coverage-measurement)
* [Code coverage measurement](#code-coverage-measurement)
* [Linters](#linters)
* [Type hints checks](#type-hints-checks)
* [Linters](#linters)
* [Ruff](#ruff)
* [Pylint](#pylint)
* [Security checks](#security-checks)
* [Code style](#code-style)
* [Formatting rules](#formatting-rules)
* [Docstrings style](#docstrings-style)

<!-- vim-markdown-toc -->
Expand Down Expand Up @@ -107,12 +110,19 @@ because the copied file is an executable script (so from GIT point of view it is
to enable it automatically).


### Code coverage measurement
## Code coverage measurement

During testing, code coverage is measured. If the coverage is below defined threshold (see `pyproject.toml` settings for actual value stored in section `[tool.coverage.report]`), tests will fail. We measured and checked code coverage in order to be able to develop software with high quality.

Code coverage reports are generated in JSON and also in format compatible with [_JUnit_ test automation framework](https://junit.org/junit5/). It is also possible to start `make coverage-report` to generate code coverage reports in the form of interactive HTML pages. These pages are stored in the `htmlcov` subdirectory. Just open the index page from this subdirectory in your web browser.


Comment on lines +113 to +119
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify coverage report formats match what CI actually produces.

Text states “JSON and JUnit-compatible” coverage outputs. Coverage.py typically emits XML/HTML; JUnit is usually for test results, not coverage.

Consider aligning this with the Makefile/CI. To check:

🏁 Script executed:

#!/bin/bash
# Inspect coverage targets and report outputs
rg -nC2 -i 'coverage|cov' Makefile **/.github/workflows/*.yml **/pyproject.toml

Length of output: 1024

Clarify coverage vs. JUnit outputs.

Makefile shows coverage is emitted as JSON (--cov-report "json:...") and pytest writes JUnit XML as test-result files (--junit-xml). CONTRIBUTING.md (lines 113–119) is misleading — JUnit XML is not a coverage report. Update CONTRIBUTING.md to state coverage reports are JSON and that test results are produced in JUnit XML, and either add/point to pyproject.toml if it contains the coverage threshold or remove/replace that reference (Makefile unit/integration targets are the authoritative source).

🤖 Prompt for AI Agents 
In CONTRIBUTING.md around lines 113–119, the text incorrectly implies JUnit XML
is a coverage report; update the paragraph to state that coverage reports are
generated as JSON (and HTML via make coverage-report/htmlcov) and that pytest
writes JUnit XML files containing test results (not coverage), and then either
point readers to pyproject.toml for the coverage threshold or remove that
reference and instead point to the Makefile (unit/integration targets) as the
authoritative source for how coverage and test result outputs are produced; make
the wording concise and replace the misleading sentence accordingly.


## Linters

_Black_, _Ruff_, Pyright, _Pylint_, __Pydocstyle__, __Mypy__, and __Bandit__ tools are used as linters. There are a bunch of linter rules enabled for this repository. All of them are specified in `pyproject.toml`, such us in sections `[tool.ruff]` and `[tool.pylint."MESSAGES CONTROL"]`. Some specific rules can be disabled using `ignore` parameter (empty now).


### Type hints checks

It is possible to check if type hints added into the code are correct and whether assignments, function calls etc. use values of the right type. This check is invoked by following command:
Expand All @@ -126,9 +136,7 @@ Subsequent runs are much faster thanks to the cache that Mypy uses. This check
is part of a CI job that verifies sources.


### Linters

_Black_, _Ruff_, Pyright, and _Pylint_ tools are used as linters. There are a bunch of linter rules enabled for this repository. All of them are specified in `pyproject.toml`, such us in sections `[tool.ruff]` and `[tool.pylint."MESSAGES CONTROL"]`. Some specific rules can be disabled using `ignore` parameter (empty now).
### Ruff

List of all _Ruff_ rules recognized by Ruff can be retrieved by:

Expand All @@ -145,6 +153,8 @@ Ruff rules can be disabled in source code (for given line or block) by using spe
# noqa: E501
```

### Pylint

List of all _Pylint_ rules can be retrieved by:

```
Expand Down Expand Up @@ -173,6 +183,10 @@ make security-check

## Code style

### Formatting rules

Code formatting rules are checked by __Black__. More info can be found on [https://black.readthedocs.io/en/stable/](https://black.readthedocs.io/en/stable/).

### Docstrings style
We are using [Google's docstring style](https://google.github.io/styleguide/pyguide.html).

Expand Down
2 changes: 2 additions & 0 deletions docs/testing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ Currently code coverage threshold for integration tests is set to 60%. This valu

As specified in Definition of Done, new changes need to be covered by tests.

[NOTE] Please do not mix pytest and unittest modules in new tests. It is preferred to use pytest in every tests, which will help the team to maintain consistency.

Comment on lines +72 to +73
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Clarify pytest vs unittest guidance; fix grammar.

Current note conflicts with earlier guidance that unittest is used for mocking/patching and has grammar issues.

Apply:

-[NOTE] Please do not mix pytest and unittest modules in new tests. It is preferred to use pytest in every tests, which will help the team to maintain consistency.
+[NOTE] Prefer pytest for all new tests and avoid writing unittest.TestCase-based tests. Using unittest.mock within pytest tests is fine. This consistency helps the team maintain the suite.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[NOTE] Please do not mix pytest and unittest modules in new tests. It is preferred to use pytest in every tests, which will help the team to maintain consistency.
[NOTE] Prefer pytest for all new tests and avoid writing unittest.TestCase-based tests. Using unittest.mock within pytest tests is fine. This consistency helps the team maintain the suite.
🤖 Prompt for AI Agents 
In docs/testing.md around lines 72 to 73, the current note both has grammar
problems and conflicts with earlier guidance about using unittest for
mocking/patching; replace it with a clear, grammatically-correct sentence that
says: prefer writing tests with pytest for consistency, do not mix pytest and
unittest frameworks within the same test file, and it is acceptable to use
unittest.mock (or the mock library) for mocking/patching when needed. Ensure the
wording is concise and consistent with the earlier section about mocking.



### Unit tests structure
Expand Down