weiqingy opened a new pull request, #689:
URL: https://github.com/apache/flink-agents/pull/689

   ### Purpose of change
   
   Follow-up to PR #686 review. @wenjin272 [pointed 
out](https://github.com/apache/flink-agents/pull/686#issuecomment-4474060000) 
that `os.environ[X] = Y` mutations at module scope in e2e test files leak into 
the rest of the pytest process at collection time. The mechanism:
   
   - `tools/ut.sh` runs `pytest flink_agents -k "not e2e_tests"`.
   - `-k` only filters which tests **execute**, not which files are 
**collected**. Pytest imports every matching test file during the collection 
phase.
   - Importing an e2e module that does `os.environ[X] = Y` at module scope 
executes the assignment as a side effect, polluting `X` for every 
later-collected test (e.g. the `integrations/` tests).
   
   PR #686 fixed this for `OLLAMA_EMBEDDING_MODEL`. This PR addresses the 
chat-model-side env vars — the same pattern that @wenjin272 mentioned in the 
"all environment variable leakage issues" follow-up.
   
   ### Files fixed
   
   | File | Env var(s) moved out of module scope |
   |---|---|
   | `e2e_tests_resource_cross_language/chat_model_cross_language_test.py:44` | 
`OLLAMA_CHAT_MODEL` |
   | `e2e_tests_resource_cross_language/yaml_cross_language_test.py:72` | 
`OLLAMA_CHAT_MODEL` |
   | `e2e_tests_integration/chat_model_integration_test.py:32,34,36,38,42` | 
`TONGYI_CHAT_MODEL`, `OLLAMA_CHAT_MODEL`, `OPENAI_CHAT_MODEL`, 
`AZURE_OPENAI_CHAT_MODEL`, `AZURE_OPENAI_API_VERSION` |
   | `e2e_tests_integration/react_agent_test.py:54` | `OLLAMA_CHAT_MODEL` |
   
   ### Approach
   
   Used `monkeypatch.setenv(...)` inside the test function bodies (one of three 
approaches @wenjin272 listed in the PR #686 review). This:
   
   - Sets the env var only when the test actually runs (no collection-time 
pollution)
   - Auto-restores the original value after the test (no inter-test leakage 
either)
   - Preserves runtime propagation to the Flink subprocess
   
   In `chat_model_integration_test.py` also converted the 
already-inside-function `os.environ["MODEL_PROVIDER"] = model_provider` to 
`monkeypatch.setenv` for consistency — otherwise the `parametrize` iterations 
(Ollama / Tongyi / OpenAI / AzureOpenAI) would leak `MODEL_PROVIDER` between 
runs.
   
   ### Tests
   
   - `./tools/lint.sh -c` clean
   - AST parse of all 4 files clean
   - The actual proof comes when CI runs and the chat-model tests no longer 
pollute the integrations test process. (No new test added because the 
underlying behavior — running an e2e test — is unchanged; the fix is purely 
about test-process hygiene.)
   
   ### API
   
   No API changes — test files only.
   
   ### Documentation
   
   - [ ] `doc-needed`
   - [x] `doc-not-needed`
   - [ ] `doc-included`
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to