Feature/#25 - Add command line option --script-languages#28
Conversation
62fe971 to
ee4237b
Compare
ee4237b to
e0019d2
Compare
e0019d2 to
6ebedfd
Compare
6ebedfd to
ed55c55
Compare
3e7b671 to
dd40abf
Compare
| """ | ||
| The operation will be skipped if SCRIPT_LANGUAGES_OPTION is defined. | ||
| """ | ||
| if request.config.getoption(SCRIPT_LANGUAGES_OPTION): |
There was a problem hiding this comment.
we need a test for this interaction
| result.assert_outcomes(passed=2, skipped=0) | ||
|
|
||
|
|
||
| def test_pytest_slc_skip(pytester): |
There was a problem hiding this comment.
got moved to test/integration/skip_test.py
| pytest_plugins = ["pytester"] | ||
|
|
||
|
|
||
| def test_empty_value_for_script_languages_raises_wrapper(pytester): |
There was a problem hiding this comment.
we need another test when the option is not set at all, how the script_languages fixtures react
|
To use Codex here, create a Codex account and connect to github. |
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd40abf231
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pyexasol_connection_, LanguageActivationLevel.Session | ||
| ) | ||
| set_script_languages(pyexasol_connection_, script_languages_) | ||
| yield |
There was a problem hiding this comment.
Ensure script languages are restored after failures
When a test using activate_script_languages_for_module or activate_script_languages_for_session fails or errors, contextlib throws that exception into this bare yield, so the reset call after it is skipped. That leaves the shared pyexasol_connection session configured with the override for later tests or teardown; wrap the yield in try/finally so the original SCRIPT_LANGUAGES value is always restored.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@chatgpt-codex-connector In a yield fixture, the code after the yield is always executed as teardown, even if the test fails or raises an exception.
There was a problem hiding this comment.
To use Codex here, create an environment for this repo.
There was a problem hiding this comment.
Summary
- Wrapped
activate_script_languagesteardown in atry/finallyblock so the previousSCRIPT_LANGUAGESvalue is restored even when the context body raises. exasol/pytest_slc/init.pyL225-L230 - Added a unit test that exercises an exception inside
activate_script_languagesand verifies the original script language setting is restored. test/unit/script_languages_unit_test.pyL8-L29 - Committed the changes on the current branch with commit
78482ceand created a PR titled “Fix script language restoration after context errors”.
Testing
- ✅
poetry run python -m black --target-version py312 --check exasol/pytest_slc/__init__.py test/unit/script_languages_unit_test.py - ✅
poetry run pytest test/unit ⚠️ poetry run pytest test/unit test/integration/script_languages_test.py— integration checks failed because local Docker/ITDE database startup is unavailable in this environment.
There was a problem hiding this comment.
In a yield fixture, the code after the yield is always executed as teardown, even if the test fails or raises an exception. @codex address that feedback
There was a problem hiding this comment.
Summary
- Wrapped
activate_script_languagesteardown in atry/finallyso direct context-manager usage restores the previousSCRIPT_LANGUAGESvalue even when the body raises. exasol/pytest_slc/init.pyL217-L230 - Added a unit test that simulates an exception inside
activate_script_languagesand verifies the temporary value is reset to the previous setting. test/unit/script_languages_unit_test.pyL14-L31 - Committed the changes on the current branch with commit
9204c47and created a PR titled “Fix script language restoration after context errors”.
Testing
- ✅
poetry run python -m black --target-version py310 --check exasol/pytest_slc/__init__.py test/unit/script_languages_unit_test.py - ✅
poetry run pytest test/unit
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a096feafa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pyexasol_connection_, LanguageActivationLevel.Session | ||
| ) | ||
| set_script_languages(pyexasol_connection_, script_languages_) | ||
| yield |
There was a problem hiding this comment.
Restore script languages from a finally block
When callers use activate_script_languages directly as a context manager and the body raises, that exception is thrown back at this yield, so the reset on the next line is skipped and the shared pyexasol_connection can keep the temporary SCRIPT_LANGUAGES value for later tests. Wrap the yield in try/finally so failing UDF tests do not contaminate the rest of the run.
Useful? React with 👍 / 👎.
Signed-off-by: Steffen Pankratz <steffen.pankratz@exasol.com>
Signed-off-by: Steffen Pankratz <steffen.pankratz@exasol.com>
Signed-off-by: Steffen Pankratz <steffen.pankratz@exasol.com>
Signed-off-by: Steffen Pankratz <steffen.pankratz@exasol.com>
Signed-off-by: Steffen Pankratz <steffen.pankratz@exasol.com>
Signed-off-by: Steffen Pankratz <steffen.pankratz@exasol.com>
Signed-off-by: Steffen Pankratz <steffen.pankratz@exasol.com>
20bae72 to
81fe648
Compare
|


Fixes #25