Skip to content

Integrate tests into docker workflows #366

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 5 commits into from
May 21, 2025
Merged

Integrate tests into docker workflows #366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
97985ec
Integrate tests step in existing workflows
rhanka May 19, 2025
f45af24
add 20 basic unit tests
rhanka May 21, 2025
a08ed5c
consolidate tests for numpy further migration
rhanka May 21, 2025
207b2a5
add 6 new tests
rhanka May 21, 2025
1706c7f
add 3 tests for pandas future migration
rhanka May 21, 2025
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
7 changes: 6 additions & 1 deletion .github/workflows/pull.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ jobs:
id: extract_branch
- name: build
if: success()
run: make version backend-docker-check GIT_BRANCH=$GIT_BRANCH || ( make backend-build GIT_BRANCH=$GIT_BRANCH && make backend backend-stop GIT_BRANCH=$GIT_BRANCH)
run: make version backend-docker-check GIT_BRANCH=$GIT_BRANCH || ( make backend-build GIT_BRANCH=$GIT_BRANCH && make backend GIT_BRANCH=$GIT_BRANCH)
env:
GIT_BRANCH: ${{ steps.extract_branch.outputs.branch }}
- name: tests
if: success()
run: make tests backend-stop GIT_BRANCH=$GIT_BRANCH
Comment on lines +20 to +22
Copy link

@coderabbitai coderabbitai bot May 19, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Separate test execution from cleanup to ensure teardown on failures
By chaining make tests backend-stop, the backend will only be stopped if tests pass—failing tests will leave containers running. It’s more robust to split into two steps: one for tests and another (with if: always()) for cleanup.

@@ -20,5 +20,12 @@
       env:
         GIT_BRANCH: ${{ steps.extract_branch.outputs.branch }}
-      - name: tests
-        if: success()
-        run: make tests backend-stop GIT_BRANCH=$GIT_BRANCH
+      - name: Run tests
+        run: make tests GIT_BRANCH=$GIT_BRANCH
+        env:
+          GIT_BRANCH: ${{ steps.extract_branch.outputs.branch }}
+
+      - name: Teardown backend
+        if: always()
+        run: make backend-stop GIT_BRANCH=$GIT_BRANCH
+        env:
+          GIT_BRANCH: ${{ steps.extract_branch.outputs.branch }}
📝 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
- name: tests
if: success()
run: make tests backend-stop GIT_BRANCH=$GIT_BRANCH
env:
GIT_BRANCH: ${{ steps.extract_branch.outputs.branch }}
- name: Run tests
run: make tests GIT_BRANCH=$GIT_BRANCH
env:
GIT_BRANCH: ${{ steps.extract_branch.outputs.branch }}
- name: Teardown backend
if: always()
run: make backend-stop GIT_BRANCH=$GIT_BRANCH
env:
GIT_BRANCH: ${{ steps.extract_branch.outputs.branch }}
🤖 Prompt for AI Agents 
In .github/workflows/pull.yml around lines 20 to 22, the current step runs tests
and backend-stop together, causing backend-stop to run only if tests succeed. To
fix this, split this into two separate steps: one step to run tests without
backend-stop, and a following step to run backend-stop with the condition `if:
always()` to ensure cleanup runs regardless of test success or failure.

env:
GIT_BRANCH: ${{ steps.extract_branch.outputs.branch }}
5 changes: 5 additions & 0 deletions .github/workflows/push.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ jobs:
run: make version backend-docker-check GIT_BRANCH=$GIT_BRANCH || ( make backend-build GIT_BRANCH=$GIT_BRANCH && make backend backend-stop GIT_BRANCH=$GIT_BRANCH)
env:
GIT_BRANCH: ${{ steps.extract_branch.outputs.branch }}
- name: tests
if: success()
run: make tests
env:
GIT_BRANCH: ${{ steps.extract_branch.outputs.branch }}
- name: publish
if: success()
run: |
Expand Down
5 changes: 4 additions & 1 deletion Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ RUN mkdir -p code\
conf/run\
log\
referential_data\
matchID_test/\
tests\
upload

################################
Expand All @@ -41,11 +41,13 @@ WORKDIR /${APP}
COPY code/ code/
COPY conf/ conf/
COPY referential_data/ referential_data/
COPY tests/ tests/

VOLUME /${app_path}/projects
VOLUME /${app_path}/log
VOLUME /${app_path}/models
VOLUME /${app_path}/upload
VOLUME /${app_path}/tests
Copy link

@coderabbitai coderabbitai bot May 20, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Undefined environment variable used for volume path
The directive uses ${app_path} which isn’t declared (you only have ARG APP / ENV APP). This will cause a build error. Update it to use ${APP}, or define app_path earlier.

- VOLUME /${app_path}/tests
+ VOLUME /${APP}/tests
🤖 Prompt for AI Agents 
In Dockerfile at line 50, the VOLUME directive uses an undefined environment
variable `${app_path}`, which causes a build error. Replace `${app_path}` with
the correctly defined environment variable `${APP}` to match the existing ENV
declaration, or alternatively define `app_path` earlier in the Dockerfile before
this line.


EXPOSE ${BACKEND_PORT}

Expand All @@ -68,6 +70,7 @@ VOLUME /${APP}/referential_data
VOLUME /${APP}/log
VOLUME /${APP}/models
VOLUME /${APP}/upload
VOLUME /${APP}/tests

EXPOSE ${BACKEND_PORT}

Expand Down
6 changes: 6 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export GITHUB_OAUTH_SECRET=203010f81158d3ceab0297a213e80bc0fbfe7f8e
export BACKEND := $(shell pwd)
export UPLOAD=${BACKEND}/upload
export PROJECTS=${BACKEND}/projects
export TESTS=${BACKEND}/tests
export EXAMPLES=${BACKEND}/../examples
export TUTORIAL=${BACKEND}/../tutorial
export MODELS=${BACKEND}/models
Expand Down Expand Up @@ -696,3 +697,8 @@ deploy-remote: config deploy-remote-instance deploy-remote-services deploy-remot

clean-remote:
@make -C ${APP_PATH}/${GIT_TOOLS} remote-clean ${MAKEOVERRIDES} > /dev/null 2>&1 || true

tests:
@docker exec -i ${USE_TTY} ${DC_PREFIX}-${APP} pytest -q -W "ignore::DeprecationWarning" -W "ignore::FutureWarning"

.PHONY: tests
11 changes: 11 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,14 @@ If you want to contribute to the developpement, you'll be able to fork the repo
```
make start-dev
```

### Running tests
Install dependencies and run tests with Make:
```bash
make tests
```
You can also install the requirements and run pytest directly:
```bash
pip install -r requirements.txt
pytest
```
1 change: 1 addition & 0 deletions docker-compose-dev.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ services:
- ${BACKEND}/code/:/${APP_GROUP}/code/
- ${BACKEND}/referential_data/:/${APP_GROUP}/referential_data/
- ${BACKEND}/conf/:/${APP_GROUP}/conf/
- ${BACKEND}/tests/:/${APP_GROUP}/tests/
3 changes: 2 additions & 1 deletion requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@ typing
Werkzeug==3.0.3
rsa>=4.7 # not directly required, pinned by Snyk to avoid a vulnerability
zipp>=3.19.1 # not directly required, pinned by Snyk to avoid a vulnerability
pyarrow>=14.0.1
pyarrow>=14.0.1
pytest
Loading
Loading