Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What's up with disabled_test_converter_arguments? #20

Open
AlexWaygood opened this issue Aug 6, 2023 · 3 comments
Open

What's up with disabled_test_converter_arguments? #20

AlexWaygood opened this issue Aug 6, 2023 · 3 comments

Comments

@AlexWaygood
Copy link

AlexWaygood commented Aug 6, 2023

This method in test_clinic.py is never executed, since its name does not begin with test_, and it is never called from any methods whose names do begin with test_:

def disabled_test_converter_arguments(self):
function = self.parse_function("""
module os
os.access
path: path_t(allow_fd=1)
""")
p = function.parameters['path']
self.assertEqual(1, p.converter.args['allow_fd'])

If I prepend test_ to the method name, it fails:

Traceback:
======================================================================
ERROR: test_disabled_test_converter_arguments (test.test_clinic.ClinicParserTest.test_disabled_test_converter_arguments)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\alexw\coding\argclinic-python\Lib\test\test_clinic.py", line 986, in test_disabled_test_converter_arguments
    function = self.parse_function("""
               ^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\alexw\coding\argclinic-python\Lib\test\test_clinic.py", line 858, in parse_function
    block = self.parse(text)
            ^^^^^^^^^^^^^^^^
  File "C:\Users\alexw\coding\argclinic-python\Lib\test\test_clinic.py", line 854, in parse
    parser.parse(block)
  File "C:\Users\alexw\coding\argclinic-python\Tools\clinic\clinic.py", line 4617, in parse
    self.state(line)
  File "C:\Users\alexw\coding\argclinic-python\Tools\clinic\clinic.py", line 4870, in state_parameters_start
    return self.next(self.state_parameter, line)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\alexw\coding\argclinic-python\Tools\clinic\clinic.py", line 4656, in next
    self.state(line)
  File "C:\Users\alexw\coding\argclinic-python\Tools\clinic\clinic.py", line 4919, in state_parameter
    self.parse_parameter(param)
  File "C:\Users\alexw\coding\argclinic-python\Tools\clinic\clinic.py", line 5139, in parse_parameter
    fail(f'{name!r} is not a valid {legacy_str}converter')
  File "C:\Users\alexw\coding\argclinic-python\Tools\clinic\clinic.py", line 208, in fail
    warn_or_fail(*args, filename=filename, line_number=line_number, fail=True)
  File "C:\Users\alexw\coding\argclinic-python\Tools\clinic\clinic.py", line 191, in warn_or_fail
    raise error
clinic.ClinicError: 'path_t' is not a valid converter

----------------------------------------------------------------------

It seems like this method was introduced in the original commit adding argument clinic to CPython.

Should we just delete it? Fix it?

@erlend-aasland
Copy link

There's a path_t converter in Modules/posixmodule.c. Perhaps it was a test for an early variant of clinic that included more of those converters?

@AlexWaygood
Copy link
Author

There's a path_t converter in Modules/posixmodule.c. Perhaps it was a test for an early variant of clinic that included more of those converters?

Sounds... plausible!

@erlend-aasland
Copy link

Are there similar tests? If not, we could borrow the code in posixmodule.c and make it a test in ClinicWholeFileTest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants