Issues/172 #188

michaelwaves · 2025-10-28T19:26:23Z

A start to integrating pytest

-add pytest and dependencies to requirements.in
-add instructions to readme
-add conftest.py
-add test_ping in test_app.py

Dev

tagging the ray image in ga workflow

bug fix with non serializable result

pickle status result

Connect to object store in coordinator

Dev

test_app.py tests ping api route note: fastapi TestClient(app) doesn't work due to fastapi mounting socketio you must cd to the src/services/api directory before running pytest (added this to readme) this is currently in a draft state and should not be merged

Dev

Alert users and purge requests for processors that fail to deploy (in…

Use Torch CPU in our docker builds (ndif-team#191)

add pandas to whitelisted modules

MichaelRipa · 2025-11-10T20:07:30Z

Thanks for your contributions! I'm going to review this in more detail later this week, but wanted to give a few initial comments:

It's not documented anywhere yet, but typically we make PR's into dev branch first (I changed this for you)
I'd like to have these tests organized under a unit subdirectory, and have a way to be able to specify only running unit tests. It's somewhat trivial, but at some point, we'll have integrated / end-to-end tests so it would have having things already organized 🙂

Dev

add gitignore

MichaelRipa · 2025-11-20T19:46:10Z

README.md


+For more comprehensive testing, install pytest
+```sh
+conda activate ndif-dev


Instead of assuming a package manager and instructing them how to install it, just directly link to pytest docs.

MichaelRipa · 2025-11-20T19:51:30Z

src/services/api/tests/unit/test_queue_util.py

@@ -0,0 +1,28 @@
+import time
+from functools import lru_cache
+from src.queue.util import cache_maintainer


This helper function is getting removed in #210

hmmm it seems like there are a lot of changes happening to queue/util.py, maybe we should just delete test_queue_util.py for now?

MichaelRipa · 2025-11-20T20:12:51Z

src/services/api/README.md

+
+## Testing
+
+install dependencies from requirements.in, ideally in a virtual environment


There's also the dependencies in the pyproject.toml

MichaelRipa · 2025-11-20T20:22:05Z

src/services/api/tests/unit/test_providers_util.py

@@ -0,0 +1,10 @@
+from unittest.mock import MagicMock, patch


The repo structure is a little confusing, but this technically corresponds to src/common, so this test isn't needed here.

Ah I see. I removed the file for now since there are no other functions to test in providers/util.py. Are there any api specific utils we should test or is this fine for now?

MichaelRipa · 2025-11-20T20:23:49Z

src/services/api/pyproject.toml

+[project]
+name = "api"
+version = "0.1.0"
+requires-python = ">=3.10"


Technically right now we only run NDIF with python 3.12

MichaelRipa · 2025-11-20T20:24:52Z

src/services/api/pyproject.toml

+pythonpath = ["."]
+
+[dependency-groups]
+dev = [


I think "tests" would be a more appropriate dependency group name

MichaelRipa · 2025-11-20T20:26:55Z

src/services/api/tests/unit/conftest.py

@@ -0,0 +1,13 @@
+import os
+os.environ["DEV_MODE"] = "true"


You set this here, but also directly in some of the tests, was this intentional?

Yes, the two tests were specifically testing the behavior in dev mode so I wanted it to be explicit. The top level DEV_MODE is just to avoid start the db connection in db.py. In e2e testing we might want to set this to be false with a separate conftest.py

MichaelRipa

Great progress! I cloned and tested this out locally, and it seemed to work once I installed all the dependencies. I added some comments for review, once they are addressed, it'll be close to ready for merge 🙂

> > rename dev => test depedency group > requires-python = "3.12"

MichaelRipa · 2025-11-21T19:22:05Z

Looks good to me. Will just get a second pair of eyes from @JadenFiotto-Kaufman

JadenFiotto-Kaufman and others added 15 commits October 24, 2025 15:53

Merge pull request ndif-team#182 from ndif-team/dev

0af2166

Dev

Merge pull request ndif-team#183 from ndif-team/dev

d6d7c91

tagging the ray image in ga workflow

Merge pull request ndif-team#184 from ndif-team/dev

d8ea660

bug fix with non serializable result

Merge pull request ndif-team#185 from ndif-team/dev

142768a

pickle status result

Merge pull request ndif-team#186 from ndif-team/dev

a1b66fa

Connect to object store in coordinator

Merge pull request ndif-team#187 from ndif-team/dev

794fd43

Dev

install and configure pytest for api service

3e99ef6

pin pytest version

28cd6ec

add test_ping

737e2d7

test_app.py tests ping api route note: fastapi TestClient(app) doesn't work due to fastapi mounting socketio you must cd to the src/services/api directory before running pytest (added this to readme) this is currently in a draft state and should not be merged

Merge pull request ndif-team#189 from ndif-team/dev

374074d

Dev

update readme

26ebf94

Merge pull request ndif-team#190 from ndif-team/dev

b52ed01

Alert users and purge requests for processors that fail to deploy (in…

Merge pull request ndif-team#192 from ndif-team/dev

43b32f2

Use Torch CPU in our docker builds (ndif-team#191)

add pandas to whitelisted modules

5c5ce15

Merge pull request ndif-team#193 from michaelwaves/nnsight_issue/536

ec4db0d

add pandas to whitelisted modules

MichaelRipa changed the base branch from main to dev November 10, 2025 19:59

JadenFiotto-Kaufman and others added 9 commits November 13, 2025 13:50

Merge pull request ndif-team#205 from ndif-team/dev

72b4975

Dev

move pytest dependencies to pyproject.toml

9fa1cb8

add gitignore

move all tests to unit subdirectory for future e2e testing

9ae7c6b

add test_dependencies.py

b373667

add test_cache_maintainer in test_queue_util.py

3b07d7b

add test_providers_util.py

8ed0d2b

Merge branch 'ndif-team:main' into issues/172

5abcd46

add readme

5a699c9

update readme

d8cc7b9

michaelwaves marked this pull request as ready for review November 17, 2025 02:47

michaelwaves added 2 commits November 18, 2025 18:26

remove uv.lock

46b8b57

update readme to remove uv and make pytest commands generic

833fdef

MichaelRipa reviewed Nov 20, 2025

View reviewed changes

update README.md to point to pytest docs

4165228

MichaelRipa reviewed Nov 20, 2025

View reviewed changes

update api service README.md to include pyproject.toml deps

5eb4d09

MichaelRipa reviewed Nov 20, 2025

View reviewed changes

MichaelRipa requested changes Nov 20, 2025

View reviewed changes

michaelwaves added 4 commits November 20, 2025 21:43

update pyproject.toml

aaa7b7f

> > rename dev => test depedency group > requires-python = "3.12"

remove test_queue_util.py

47ef6d7

remove unused asyncio in test_dependencies.py

530c29e

remove test_provider_util.py

74e0241

MichaelRipa requested a review from JadenFiotto-Kaufman November 21, 2025 19:21


		## Testing

		install dependencies from requirements.in, ideally in a virtual environment

Issues/172 #188

Are you sure you want to change the base?

Issues/172 #188

Uh oh!

Conversation

michaelwaves commented Oct 28, 2025

Uh oh!

MichaelRipa commented Nov 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaelRipa left a comment

Choose a reason for hiding this comment

Uh oh!

MichaelRipa commented Nov 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants