Skip to content

Conversation

@pygarap
Copy link
Contributor

@pygarap pygarap commented Dec 16, 2025

Important

As modest is deprecated, these changes clarify the distinction between the two parser engines in the codebase and make the test suites easier to maintain.

This pull request splits and refactors the test suite for the Selectolax HTML parsers. It introduces a new, dedicated test file for the Lexbor parser (test_lexbor_parser.py) and renames and updates the previous shared test file to focus exclusively on the Modest parser (test_modest_parser.py). The refactored tests for the Modest parser remove all references to the Lexbor parser, eliminate parameterized tests that previously ran against both engines, and simplify the test implementations.

The most important changes are:

Test suite reorganization and separation:

  • Created a new file test_lexbor_parser.py containing comprehensive tests specifically for the Lexbor parser, including tests for concurrency, Unicode handling, malformed data, and API edge cases.
  • Renamed test_parser.py to test_modest_parser.py and refactored it to focus solely on the Modest parser (HTMLParser), removing all Lexbor-specific imports and logic.

Refactoring and simplification of Modest parser tests:

  • Removed parameterization over both parsers and updated all test cases to use only HTMLParser instead of supporting both HTMLParser and LexborHTMLParser. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]
  • Simplified test assertions and removed type checks and logic that depended on Lexbor-specific classes or exceptions. [1] [2]

Removal of Lexbor-specific tests from Modest suite:

  • Deleted tests from the Modest parser suite that were only relevant to the Lexbor parser, such as those for Lexbor-specific error handling, pseudo-class selectors, and tag creation helpers. [1] [2]

These changes clarify the distinction between the two parser engines in the codebase and make the test suites easier to maintain.

…parser.py` for better code organization and maintainability.
Copilot AI review requested due to automatic review settings December 16, 2025 16:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request separates the previously shared test suite for Selectolax parsers into two distinct test files: one for the deprecated Modest parser (test_modest_parser.py) and one for the Lexbor parser (test_lexbor_parser.py). This separation improves maintainability and clarifies the testing scope for each parser engine.

Key changes:

  • Created dedicated test suite for Lexbor parser with engine-specific tests
  • Refactored Modest parser tests to remove all Lexbor dependencies and parameterization
  • Eliminated dual-parser test patterns in favor of single-parser focused tests

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
tests/test_modest_parser.py Removed Lexbor imports, parameterized tests, and Lexbor-specific test cases; simplified all tests to use only HTMLParser
tests/test_lexbor_parser.py New file containing comprehensive Lexbor parser tests including concurrency, Unicode handling, pseudo-class selectors, and error handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

def test_parser(parser):
html = parser("")
assert isinstance(html, parser)
def test_HTMLParser():
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function name uses PascalCase for 'HTMLParser' which is inconsistent with Python naming conventions. Consider renaming to 'test_html_parser' to follow snake_case convention for function names.

Suggested change
def test_HTMLParser():
def test_html_parser():

Copilot uses AI. Check for mistakes.
from selectolax.lexbor import LexborHTMLParser, LexborNode, SelectolaxError, create_tag

"""
We'are testing only our own code.
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'We'are' to 'We're'.

Suggested change
We'are testing only our own code.
We're testing only our own code.

Copilot uses AI. Check for mistakes.
from selectolax.lexbor import LexborHTMLParser, LexborNode, SelectolaxError, create_tag

"""
We'are testing only our own code.
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'We'are' to 'We're'.

Suggested change
We'are testing only our own code.
We're testing only our own code.

Copilot uses AI. Check for mistakes.
@rushter
Copy link
Owner

rushter commented Dec 16, 2025

You are duplicating tests for no apparent reason.
Also, you add the burden of reviewing it to me. AI did this change for you, but I have to manually check each test change for no benefits for the project.
Modest is here for a few years, no one gonna stop it using right away.

@pygarap pygarap closed this Dec 17, 2025
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

Successfully merging this pull request may close these issues.

2 participants