Skip to content

OLS-1946: Fix contributing.md for uv #269

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
tisnik merged 1 commit into from
Jul 23, 2025
Merged

OLS-1946: Fix contributing.md for uv #269

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
8 changes: 4 additions & 4 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,15 @@ git clone https://github.com/YOUR-GIT-PROFILE/lightspeed-stack.git
cd lightspeed-stack

# setup your devel environment with uv
uv install -G dev
uv sync --group dev

# Now you can run test commands trough make targets, or prefix the rest of commands with `uv run`, eg. `uv run make test`
# Now you can run test commands trough make targets, or prefix the rest of commands with `uv run`, eg. `uv run make test` or do `uv venv`, which creates virtual environment and prints activation command, and run commands inside venv.

# run unit tests
make unit-tests
make test-unit

# run integration tests
make integration-tests
make test-integration
Copy link
Contributor

@umago umago Jul 23, 2025

Choose a reason for hiding this comment

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

Good catches!


# code formatting
# (this is also run automatically as part of pre-commit hook if configured)
Expand Down
4 changes: 2 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ run-data-collector: ## Run the data collector service locally
test-unit: ## Run the unit tests
@echo "Running unit tests..."
@echo "Reports will be written to ${ARTIFACT_DIR}"
COVERAGE_FILE="${ARTIFACT_DIR}/.coverage.unit" uv run pytest tests/unit --cov=src --cov-report term-missing --cov-report "json:${ARTIFACT_DIR}/coverage_unit.json" --junit-xml="${ARTIFACT_DIR}/junit_unit.xml" --cov-fail-under=60
COVERAGE_FILE="${ARTIFACT_DIR}/.coverage.unit" uv run python -m pytest tests/unit --cov=src --cov-report term-missing --cov-report "json:${ARTIFACT_DIR}/coverage_unit.json" --junit-xml="${ARTIFACT_DIR}/junit_unit.xml" --cov-fail-under=60

Comment on lines 14 to 18
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 22, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Create ${ARTIFACT_DIR} before writing reports

pytest and the coverage/junit reporters assume the target directory already exists.
If ${ARTIFACT_DIR} is missing, the run silently falls back to the project root and pollutes it with artefacts.

 test-unit: ## Run the unit tests
 	@echo "Running unit tests..."
 	@echo "Reports will be written to ${ARTIFACT_DIR}"
+	@mkdir -p "${ARTIFACT_DIR}"
 	COVERAGE_FILE="${ARTIFACT_DIR}/.coverage.unit" uv run python -m pytest tests/unit --cov=src --cov-report term-missing --cov-report "json:${ARTIFACT_DIR}/coverage_unit.json" --junit-xml="${ARTIFACT_DIR}/junit_unit.xml" --cov-fail-under=60

Apply the same pre-creation in the test-integration target.

🤖 Prompt for AI Agents 
In the Makefile around lines 14 to 18, the test-unit target writes coverage and
junit reports to ${ARTIFACT_DIR} without ensuring the directory exists, which
can cause files to be created in the project root if the directory is missing.
Fix this by adding a command to create the ${ARTIFACT_DIR} directory (e.g.,
mkdir -p ${ARTIFACT_DIR}) before running pytest in the test-unit target. Also,
apply the same directory creation step in the test-integration target to prevent
similar issues there.

test-integration: ## Run integration tests tests
@echo "Running integration tests..."
@echo "Reports will be written to ${ARTIFACT_DIR}"
COVERAGE_FILE="${ARTIFACT_DIR}/.coverage.integration" uv run pytest tests/integration --cov=src --cov-report term-missing --cov-report "json:${ARTIFACT_DIR}/coverage_integration.json" --junit-xml="${ARTIFACT_DIR}/junit_integration.xml" --cov-fail-under=10
COVERAGE_FILE="${ARTIFACT_DIR}/.coverage.integration" uv run python -m pytest tests/integration --cov=src --cov-report term-missing --cov-report "json:${ARTIFACT_DIR}/coverage_integration.json" --junit-xml="${ARTIFACT_DIR}/junit_integration.xml" --cov-fail-under=10

test-e2e: ## Run BDD tests for the service
PYTHONDONTWRITEBYTECODE=1 uv run behave --tags=-skip -D dump_errors=true @tests/e2e/test_list.txt \
Expand Down