Unit test#8
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds comprehensive unit tests for the FastAPI app, updates dependencies, and configures test-time behavior for tracing.
- Introduce tests for
/healthand/predictendpoints covering success, error, and edge cases - Update project dependencies (add
httpx,pytest,requests) - Conditionally disable OpenTelemetry tracing in tests and set
TESTING_MODEin CI
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/test_main.py | Added unit tests covering health check, prediction logic, and error scenarios |
| pyproject.toml | Added httpx, pytest, and requests to dependencies |
| app/utils/tracing_config.py | Skip tracing setup when TESTING_MODE environment variable is true |
| .github/workflows/lint-test.yml | Export TESTING_MODE=true in CI, removed MODEL_PATH export |
Comments suppressed due to low confidence (3)
.github/workflows/lint-test.yml:53
- The removal of
MODEL_PATHexport may break model loading in tests or CI; ensure the application can locate the model without this environment variable or re-add it where needed.
export MODEL_PATH=../models/model.pkl
pyproject.toml:17
- [nitpick] Consider whether
httpxis actually used in the application or tests; if not, removing unused dependencies will reduce installation size and maintenance overhead.
"httpx>=0.28.1",
pyproject.toml:41
- [nitpick] If
requestsis not leveraged by the code or tests, consider removing it to avoid unnecessary dependencies.
"requests>=2.32.4",
| """ | ||
| # --- Conditionally disable tracing for tests --- | ||
| if os.environ.get("TESTING_MODE", "false").lower() == "true": | ||
| print("TESTING_MODE is active. Skipping OpenTelemetry tracing setup.") |
There was a problem hiding this comment.
[nitpick] Using print in library code can clutter logs; consider using a logger at debug or info level for consistency with the rest of the application.
|
|
||
| assert response.status_code == 422 | ||
| response_json = response.json() | ||
| assert response_json["detail"][0]["type"] == "missing" |
There was a problem hiding this comment.
Pydantic’s error type for missing fields is typically value_error.missing, not missing; update the assertion to match the actual error type.
| assert response_json["detail"][0]["type"] == "missing" | |
| assert response_json["detail"][0]["type"] == "value_error.missing" |
| Tests the pre-prediction check for blocked customers. | ||
| """ | ||
| payload = sample_legitimate_payload.copy() | ||
| payload["CUSTOMER_ID"] = 323 |
There was a problem hiding this comment.
[nitpick] The literal 323 is a magic number; consider defining a descriptive constant (e.g., BLOCKED_CUSTOMER_ID) to make the intent clearer.
| payload["CUSTOMER_ID"] = 323 | |
| payload["CUSTOMER_ID"] = BLOCKED_CUSTOMER_ID |
| Tests the pre-prediction check for high-risk terminals. | ||
| """ | ||
| payload = sample_legitimate_payload.copy() | ||
| payload["TERMINAL_ID"] = 4692 |
There was a problem hiding this comment.
[nitpick] Using the literal 4692 obscures the test’s meaning; define a named constant (e.g., HIGH_RISK_TERMINAL_ID) for clarity.
| payload["TERMINAL_ID"] = 4692 | |
| payload["TERMINAL_ID"] = HIGH_RISK_TERMINAL_ID |
* add unit test requirements * turn off tracing when running test * update workflow
* init commit * add notebooks and data * Colab (#1) * add link to google colab * update structure * update jupyter-lab-docker * Refactor notebook structure (#2) * reconstruct structure folder * update report on chapter * update reports document (#3) * reconstruct structure folder * update report on chapter * update report * export decision tree model and notebook (#4) * fix grammar on notebooks (#5) * export decision tree model and notebook * fix grammar * Fast api app (#6) * first commit on python fast api app * deploy test-client * first success test however, need to implement custom rules and additional features to be considered * feat: add precondition rules to block suspicious customer and terminal * success docker deploy of both test and api * Monitoring (#7) * restructure * replace node-exporter, promtail with alloy * deploy tempo for tracing * ci: update Helm values.yaml and Chart.yaml to ghcr.io/phuchoang2603/realtime-credit-card-fraud-detection:v1.0.0 --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Unit test (#8) * add unit test requirements * turn off tracing when running test * update workflow * K8s monitoring (#9) * fix enviroment variable * update helm chart * bump version to 1.0.1 * ci: update Helm values.yaml and Chart.yaml to ghcr.io/phuchoang2603/realtime-credit-card-fraud-detection:v1.0.1 * add traefik and simplifies helm chart * add alloy argo cd test --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Monitoring configuration (#10) * add logging stack: alloy, loki, grafana * minor fix before argo-cd app-of-apps * Argo cd (#11) * argo-cd app of apps * fix alloy mount docker container * fix loki dns service to use rke2 * fix grafana pvc * revert to use pvc * add traefik and cert manager argocd app of apps * fix cert-manager metrics expose * update repo * update traefik load balancer ip * add prometheus chart * add tempo and finalize, i guess * wrong tempo path * try to remove loki resources config * pls works * move ingress route into unify location * in the end, still came back to kube-prometheus-stack, but only for grafana and prometheus * fix spacing * fix loki * move ingressroute config to another place * fix typo * fix typo #2 * bump traefik version * why does this keep happening to me * really angry * adapt url * reduce workload by disable prometheus operator * reorder file structure * remove foreground cascading deletion * try alloy receiver port 12345 * add alloy ingress route * update url * use metric from prometheus insteaed * fix typo * disable tls for tempo * try to disable tls * change cluster Ip to load balancer * revert back to default to ensure security * change target revison to main * bump to 1.0.2 * update document for deployment (#12) * rename to argo apps (#13) * fix workflow (#14) * fix workflow * bump to 1.0.3 * ci: update Helm values.yaml and Chart.yaml to ghcr.io/phuchoang2603/realtime-credit-card-fraud-detection:v1.0.3 --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Cloud (#15) * add finalizer * change manifest apps to proxmox * add gke terraform * separate proxmox and cloud * update docs * update value * update acme tls server * fix acme server * drop tls verify on test client * bump to v1.0.4 * ci: update Helm values.yaml and Chart.yaml to ghcr.io/phuchoang2603/realtime-credit-card-fraud-detection:v1.0.4 --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Documents (#16) * change namespace * add README * change target revision temporarily * update docs * change to main * update README * bump to 1.0.5 * format Dockerfile * fix alloy endpoint * refactor: separate prod and dev requirements.txt * refactor: move fixture into separate file * simulate await api calling for pre_condition_checks * ci: add step check code coverage * rename helm-chart to helm-charts * add helm chart for traefik and cert-manager, group into api-gateway namespace * docs: change name and graph * now using lifespan event, simpler test client too * ci: experiement remove uv pip install --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
No description provided.