Eason09053360 commented on code in PR #72291:
URL: https://github.com/apache/airflow/pull/72291#discussion_r3890297427
##########
airflow-core/tests/unit/cli/commands/test_task_command.py:
##########
@@ -215,7 +215,18 @@ def test_cli_test_with_params(self):
)
)
- def test_cli_test_with_env_vars(self):
+ @pytest.mark.parametrize(
+ ("env_var_args", "expected_foo"),
+ [
+ pytest.param([], "foo=None", id="without-env-vars"),
+ pytest.param(["--env-vars", '{"foo":"bar"}'], "foo=bar",
id="with-env-vars"),
+ ],
+ )
+ def test_cli_test_with_env_vars(self, monkeypatch, env_var_args,
expected_foo):
+ # task_test writes both keys into the real process environment and
never restores them;
+ # clear them so this case sees only what this invocation exported.
+ monkeypatch.delenv("AIRFLOW_TEST_MODE", raising=False)
+ monkeypatch.delenv("foo", raising=False)
Review Comment:
Good catch!
Applied with some adjustment: as-written fails `without-env-vars`,
because the example Dag prints `foo=sentinel` while that case asserts
`foo=None`.
Measured before/after on this test:
| | `AIRFLOW_TEST_MODE` | `foo` |
| -------- | ------------------- | ------------- |
| `delenv` | leaked `True` | leaked `bar` |
| `setenv` | clean | clean |
The sentinel also strengthens the assertion — the command now has to
overwrite a
pre-existing value rather than merely populate an absent one. Regression
detection
still holds: with the fix reverted, `without-env-vars` fails in both
parametrize orders.
--
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]
