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

4.1.1: test_selectors (tests.test_typing.TestTyping) fails #64

Closed
mtelka opened this issue Apr 26, 2023 · 8 comments
Closed

4.1.1: test_selectors (tests.test_typing.TestTyping) fails #64

mtelka opened this issue Apr 26, 2023 · 8 comments

Comments

@mtelka
Copy link

mtelka commented Apr 26, 2023

The test_selectors (tests.test_typing.TestTyping) test fails with elementpath 4.1.1, mypy 1.2.0, and Python 3.9.16:

FAIL: test_selectors (tests.test_typing.TestTyping)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "$(BUILD_DIR)/tests/test_typing.py", line 51, in test_selectors
    output_lines = self.check_mypy_output(case_path, '--strict')
  File "$(BUILD_DIR)/tests/test_typing.py", line 46, in check_mypy_output
    self.assertNotRegex(output_lines[-1], self.error_pattern, msg=output)
AssertionError: Regex matched: 'Found 3 error' matches 'Found \\d+ error' in 'Found 3 errors in 1 file (checked 1 source file)' : elementpath/etree.py:238: error: Library stubs not installed for "lxml.etree"  [import]
elementpath/etree.py:238: note: Hint: "python3 -m pip install lxml-stubs"
elementpath/etree.py:238: note: (or run "mypy --install-types" to install all missing stub packages)
elementpath/etree.py:238: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
elementpath/etree.py:238: error: Library stubs not installed for "lxml"  [import]
elementpath/etree.py:238: error: Name "etree_module" already defined on line 230  [no-redef]
Found 3 errors in 1 file (checked 1 source file)
@brunato
Copy link
Member

brunato commented Apr 28, 2023

Hi, i just tested this. Maybe you need to install lxml-stubs package.

@mtelka
Copy link
Author

mtelka commented Apr 28, 2023

The lxml-stubs is not listed in tox.ini, so if lxml-stubs is needed then please add it into tox.ini. Thank you.

@brunato
Copy link
Member

brunato commented Apr 28, 2023

The lxml-stubs is required only for mypy tests so is listed in tox.ini only for mypy-* envs.
It's also already included into requirements-dev.txt file.

@mtelka
Copy link
Author

mtelka commented Apr 28, 2023

That's strange, because I run tests basically using tox -e py39 and the command that caused the failure is this one (I added --verbose to tox.ini to see detailed output):

py39: commands[0]> python -m unittest --verbose
test_context_activation (tests.test_collations.CollationsTest) ... ok
test_context_manager_init (tests.test_collations.CollationsTest) ... ok
test_html_ascii_case_insensitive_collation (tests.test_collations.CollationsTest) ... ok
[...snip...]
test_build_node_tree_with_element_tree (tests.test_tree_builders.TreeBuildersTest) ... ok
test_selectors (tests.test_typing.TestTyping) ... FAIL
test_validate_analyzed_string (tests.test_validators.ValidatorsTest) ... ok
[...snip...]
test_xpath_error (tests.test_xpath_tokens.XPath2TokenTest) ... ok
test_xpath_error_shortcuts (tests.test_xpath_tokens.XPath2TokenTest) ... ok

======================================================================
FAIL: test_selectors (tests.test_typing.TestTyping)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "$(BUILD_DIR)/tests/test_typing.py", line 51, in test_selectors
    output_lines = self.check_mypy_output(case_path, '--strict')
  File "$(BUILD_DIR)/tests/test_typing.py", line 46, in check_mypy_output
    self.assertNotRegex(output_lines[-1], self.error_pattern, msg=output)
AssertionError: Regex matched: 'Found 3 error' matches 'Found \\d+ error' in 'Found 3 errors in 1 file (checked 1 source file)' : elementpath/etree.py:238: error: Library stubs not installed for "lxml.etree"  [import]
elementpath/etree.py:238: note: Hint: "python3 -m pip install lxml-stubs"
elementpath/etree.py:238: note: (or run "mypy --install-types" to install all missing stub packages)
elementpath/etree.py:238: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
elementpath/etree.py:238: error: Library stubs not installed for "lxml"  [import]
elementpath/etree.py:238: error: Name "etree_module" already defined on line 230  [no-redef]
Found 3 errors in 1 file (checked 1 source file)

----------------------------------------------------------------------
Ran 2404 tests in 71.354s

FAILED (failures=1, skipped=1)
py39: exit 1 (82.49 seconds) $(BUILD_DIR)> python -m unittest --verbose pid=9303
  py39: FAIL code 1 (82.57=setup[0.08]+cmd[82.49] seconds)
  evaluation failed :( (83.22 seconds)

So it looks like test_typing.py is calling mypy explicitly when it is installed. In my case it is installed.

The requirements-dev.txt file is not referenced from tox.ini, so tox won't install anything from such file before it runs tests.

@brunato
Copy link
Member

brunato commented Apr 28, 2023

Yes, maybe it depends on mypy. I'll check if there is a manner to skip test if a stub is not installed, otherwise i will add lxml-stub to common deps.

@brunato
Copy link
Member

brunato commented Apr 28, 2023

Temporary adding lxml-stub to avoid testing issues. In the future maybe a refactor of test_typing.py using mypy API.

Thank you

brunato added a commit that referenced this issue Jun 17, 2023
    - Skip tests if lxml-stubs is not installed (issue #64)
@brunato
Copy link
Member

brunato commented Jun 18, 2023

With the release v4.1.3 static typing tests have been refactored using mypy.api and are skipped if mypy or lxml-stubs are not installed.

@brunato brunato closed this as completed Jun 18, 2023
@stratakis
Copy link

Could the workaround be removed now since the relevant tests can be skipped if lxml-stubs is not installed?

Trying to update the package in Fedora where we don't have lxml-stubs packaged and elementpath started requiring it for tests deps that are derived from tox.ini (which shouldn't be the case as lxml-stubs is now optional).

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

3 participants