Skip to content

Conversation

@D3X
Copy link
Contributor

@D3X D3X commented Jun 19, 2019

When a test name is long (that might include class hierarchy and parametrization), the generated filename may exceed max supported length.
This PR truncates long names to avoid that, while preserving hashes of non-truncated names.

@BeyondEvil
Copy link
Contributor

BeyondEvil commented Jun 19, 2019

Thanks for this PR @D3X!

Do you happen to have a screenshot of what a report looks like (before and after) when using this?

What I'm looking for is that the truncation doesn't happen at the end of the name, but rather in the middle or start.

To illustrate.

I don't want these:

tests/component/datepicker_test.py::test_picker_using_mouse
tests/component/datepicker_test.py::test_picker_using_keyboard

to become:

tests/component/datepicker_test.py::test_picker_using
tests/component/datepicker_test.py::test_picker_using

but rather:

tests/.../datepicker_test.py::test_picker_using_mouse
tests/.../datepicker_test.py::test_picker_using_keyboard

or:

.../component/datepicker_test.py::test_picker_using_mouse
.../component/datepicker_test.py::test_picker_using_keyboard

Hope that makes sense! 😊

@BeyondEvil
Copy link
Contributor

Oh wait... You're talking about filename and not testname... right? 🙈

@D3X
Copy link
Contributor Author

D3X commented Jun 19, 2019

@BeyondEvil um, yes, filenames. Perhaps I should've worded that differently :)
This truncates test names when used for asset filenames. Only to avoid ending up with filenames that are longer than what the FS supports.

(force-pushed a fix for flake8 complaints)

@BeyondEvil
Copy link
Contributor

@D3X Nothing wrong with your wording, I just have to learn how to read. 😂

This is awesome! Just because I'm curious, did you see an issue where the original functionality broke?

As soon as tests pass, I'll approve! 👍

@D3X
Copy link
Contributor Author

D3X commented Jun 19, 2019

@BeyondEvil yes, our test suite was broken because of that, that's why I created this PR.

Nested test classes + long test method names + parametrization = looooong test names = OSError :)

OSError: [Errno 36] File name too long: '/home/circleci/project/selenium_html/assets/core/tests/selenium_tests/lawfirm_tests/law_firm_manual_matters/test_law_firm_manual_matters_on_business.py::TestLawFirmManualMattersOnBusiness::test_closed_law_firm_manual_matter_can_enter_accruals_for_this_month[INVOICE_VIEWS:False-NEW_LAW_FIRM_MATTERLIST:True-LAW_FIRM_ACCESS_CONTROL:True]01_0a7bf5cac9650399d9bdd7618e2d7e26.txt'

@BeyondEvil
Copy link
Contributor

@D3X WOW! That's... impressive! 😊

@BeyondEvil
Copy link
Contributor

@D3X When this is merged I'll make a hotfix release tonight or tomorrow, because this is a bug. 👍

Copy link
Contributor

@BeyondEvil BeyondEvil left a comment

Choose a reason for hiding this comment

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

LGTM!

@BeyondEvil
Copy link
Contributor

BeyondEvil commented Jun 19, 2019

I'll leave the honors to you @D3X.

Unless you take too long. ;)

@D3X
Copy link
Contributor Author

D3X commented Jun 19, 2019

@BeyondEvil I don't think I have access to merge, so go ahead :)

@BeyondEvil BeyondEvil merged commit e5125d5 into pytest-dev:master Jun 19, 2019
BeyondEvil added a commit to BeyondEvil/pytest-html that referenced this pull request Jun 19, 2019
@BeyondEvil
Copy link
Contributor

@D3X Version 1.21.1 is out with your fix! Thanks! 👍

@D3X D3X deleted the truncate-long-test-names branch June 20, 2019 08:44
@ssbarnea ssbarnea added the feature This issue/PR relates to a feature request. label Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature This issue/PR relates to a feature request.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants