Skip to content

test: skip test_security_module when ALIBABA_API_KEY is unset#2633

Merged
spomichter merged 2 commits into
release/0.0.13from
test/skipif-no-alibaba-security-module
Jun 26, 2026
Merged

test: skip test_security_module when ALIBABA_API_KEY is unset#2633
spomichter merged 2 commits into
release/0.0.13from
test/skipif-no-alibaba-security-module

Conversation

@KrishnaH96

Copy link
Copy Markdown
Contributor

What

Adds @pytest.mark.skipif_no_alibaba to test_security_module.

Why

The test reaches a person-follow path that calls Qwen VL (ALIBABA_API_KEY),
but it was only gated by skipif_no_openai. On a machine with OPENAI_API_KEY
but no ALIBABA_API_KEY, it ran and hung until the 360s timeout instead of
skipping. The skipif_no_alibaba marker already exists in conftest; this just
applies it, mirroring the existing skipif_no_openai gate.

Test

ruff check clean. With ALIBABA_API_KEY unset the test is skipped at
collection (pytest_collection_modifyitems) instead of running; with the key
set it still runs.

Contributor License Agreement

  • I have read and approved the CLA.

@KrishnaH96 KrishnaH96 marked this pull request as ready for review June 26, 2026 20:53
@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds @pytest.mark.skipif_no_alibaba to test_security_module so the test is skipped at collection time when ALIBABA_API_KEY is unset, preventing a 360-second timeout hang on machines that have OPENAI_API_KEY but not ALIBABA_API_KEY.

  • The skipif_no_alibaba marker is already registered in dimos/conftest.py and handled in pytest_collection_modifyitems, so no new infrastructure is needed — this is purely an annotation addition mirroring the existing skipif_no_openai gate.

Confidence Score: 5/5

Safe to merge — the change is a single marker annotation with no logic impact and no risk of regressions.

The only change is adding a pre-existing, well-tested skip marker to one test function. The marker is registered and handled correctly in conftest.py, and its skip logic is identical in structure to the existing skipif_no_openai gate already on the same test. No production code paths are touched.

No files require special attention.

Important Files Changed

Filename Overview
dimos/e2e_tests/test_security_module.py Adds @pytest.mark.skipif_no_alibaba marker to test_security_module; the marker already exists in conftest and the skip logic is correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[pytest collection] --> B{skipif_in_ci\nCI env set?}
    B -- yes --> SKIP1[Skip test]
    B -- no --> C{skipif_no_openai\nOPENAI_API_KEY set?}
    C -- no --> SKIP2[Skip test]
    C -- yes --> D{skipif_no_alibaba\nALIBABA_API_KEY set?}
    D -- no --> SKIP3[Skip test\n✅ NEW gate]
    D -- yes --> E{mujoco\nenvironment OK?}
    E -- no --> SKIP4[Skip test]
    E -- yes --> F[Run test_security_module\ncalls Qwen VL via ALIBABA_API_KEY]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[pytest collection] --> B{skipif_in_ci\nCI env set?}
    B -- yes --> SKIP1[Skip test]
    B -- no --> C{skipif_no_openai\nOPENAI_API_KEY set?}
    C -- no --> SKIP2[Skip test]
    C -- yes --> D{skipif_no_alibaba\nALIBABA_API_KEY set?}
    D -- no --> SKIP3[Skip test\n✅ NEW gate]
    D -- yes --> E{mujoco\nenvironment OK?}
    E -- no --> SKIP4[Skip test]
    E -- yes --> F[Run test_security_module\ncalls Qwen VL via ALIBABA_API_KEY]
Loading

Reviews (1): Last reviewed commit: "Merge branch 'release/0.0.13' into test/..." | Re-trigger Greptile

@spomichter spomichter enabled auto-merge (squash) June 26, 2026 20:55
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

@@                Coverage Diff                @@
##             release/0.0.13    #2633   +/-   ##
=================================================
  Coverage                  ?   71.01%           
=================================================
  Files                     ?      892           
  Lines                     ?    79238           
  Branches                  ?     7081           
=================================================
  Hits                      ?    56268           
  Misses                    ?    21130           
  Partials                  ?     1840           
Flag Coverage Δ
OS-ubuntu-24.04-arm 63.31% <100.00%> (?)
OS-ubuntu-latest 66.07% <100.00%> (?)
Py-3.10 66.06% <100.00%> (?)
Py-3.11 66.06% <100.00%> (?)
Py-3.12 66.07% <100.00%> (?)
Py-3.13 66.06% <100.00%> (?)
Py-3.14 66.08% <100.00%> (?)
Py-3.14t 66.07% <100.00%> (?)
SelfHosted-Large 30.03% <100.00%> (?)
SelfHosted-Linux 37.59% <100.00%> (?)
SelfHosted-macOS 36.41% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/e2e_tests/test_security_module.py 52.17% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@spomichter spomichter merged commit e056716 into release/0.0.13 Jun 26, 2026
30 checks passed
@spomichter spomichter deleted the test/skipif-no-alibaba-security-module branch June 26, 2026 21:12
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants