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

✅ Add more tests for the behaviour of rich_markup_mode #964

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

svlandeg
Copy link
Member

@svlandeg svlandeg commented Sep 2, 2024

There's a few open PRs around markdown formatting and parsing that I want to review, but first I would like to establish what exactly is the desired outcome for different docstrings and different settings of rich_markup_mode.

This PR is supposed to capture the ideal output for each test, using a pytest.mark.xfail marker for those cases that currently fail on master. Any potential fixes will then be contributed in subsequent PRs.

Comment on lines +46 to +54
@pytest.mark.parametrize(
"mode,lines",
[
("markdown", ["First line", "Line 1 Line 2 Line 3", ""]),
("rich", ["First line", "Line 1", "", "Line 2", "", "Line 3", ""]),
("none", ["First line", "Line 1", "Line 2", "Line 3", ""]),
],
)
def test_markup_mode_newline_pr815(mode: str, lines: List[str]):
Copy link
Member Author

Choose a reason for hiding this comment

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

Here, the question is mostly about the newlines: should there be one after "First line" or not?

Further, should the markdown parser output "Line 1 Line 2 Line 3" as one string, or split them into multiple strings, similar to the current behaviour for the option none?

),
],
)
def test_markup_mode_newline_issue447(mode: str, lines: List[str]):
Copy link
Member Author

Choose a reason for hiding this comment

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

Here, the splitting of the strings is different for all 3 options, with rich keep each input line as such, markdown merging everything that is not header, and none keeping all double newlines as separators.

Comment on lines +152 to +156
pytest.param(
"markdown",
["First line", "", "• 1", "• 2", "• 3", ""],
marks=pytest.mark.xfail(),
),
Copy link
Member Author

Choose a reason for hiding this comment

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

This test currently fails on master, with the output being

['First line', '', '• 1 - 2 - 3', '']

-> this happens mostly because the newlines are not parsed correctly, after which the bullet points aren't recognized correctly. PR #815 attempts to fix this.

Comment on lines +191 to +195
pytest.param(
"markdown",
["First line", "", "• 1", "• 2", "• a", "• b", "• 3", ""],
marks=pytest.mark.xfail(),
),
Copy link
Member Author

Choose a reason for hiding this comment

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

This test currently fails on master, with the output being

['First line', '', '• 1 - 2   - a   - b - 3', '']

-> this happens mostly because the newlines are not parsed correctly, after which the bullet points aren't recognized correctly. PR #815 attempts to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal repo / tests Involving the CI / test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants