BUG: fix common arg handling #45
Conversation
cpelley
force-pushed
the
FIX_ARGUMENT_PASS
branch
from
dc54a69
to
a256e0b
Compare
cpelley
force-pushed
the
FIX_ARGUMENT_PASS
branch
from
ad68f70
to
5a96dc3
Compare
cpelley
force-pushed
the
FIX_ARGUMENT_PASS
branch
from
db97abc
to
2705c6d
Compare
cpelley
force-pushed
the
FIX_ARGUMENT_PASS
branch
from
2705c6d
to
73929cd
Compare
| - name: Run pytest + coverage report gen | ||
| run: pytest --cov=dagrunner --cov-report=term --cov-report=html | tee coverage_output.txt | ||
| run: pytest --cov=dagrunner --cov-report=term --cov-report=html --ignore=dagrunner/tests/utils/logging/test_integration.py | tee coverage_output.txt; test ${PIPESTATUS[0]} -eq 0 |
There was a problem hiding this comment.
Exclude logging test in CI until #44
|
The test coverage has decreased from 84% to 78% (commit SHA: 73929cd). |
| callable_kwargs = callable_kwargs | _get_common_args_matching_signature( | ||
| callable_obj, common_kwargs, callable_kwargs.keys() | ||
| ) # based on overriding arguments |
There was a problem hiding this comment.
Changes that weren't included in #43, hidden pytest failures
| if "optional arguments:" in help_str: | ||
| # older versions of argparse use "optional arguments: instead of options:" | ||
| tar = tar.replace("options:", "optional arguments:") | ||
| assert help_str == tar |
There was a problem hiding this comment.
Supporting old and new argparse versions (not sure when this exactly changed right now). Was a cause of failures on CI and local succeeds.
| @@ -12,6 +12,24 @@ | |||
| CALLING_MOD = os.path.basename(sys.argv[0]) | |||
|
|
|||
|
|
|||
| def assert_help_str(help_str, tar): | |||
| # Remove line wraps for easier comparison | |||
There was a problem hiding this comment.
Line wrap changes between versions of argparse, so we remove them. Again a cause of failures on CI with success locally.
Safe to ignore this. Cause: ignoring logging integration tests (which don't work in CI right now). Will be re-enabled in #39. |
#43 didn't quite fix the issue it was intended to address.
I must have left changes locally (i.e. not pushed up all changes to github). CI pytest failures were being hidden by our use of pipe.
test ${PIPESTATUS[0]} -eq 0in the CI workflow).dagrunner/tests/utils/test_function_to_parse.py.Issues