Skip to content

Conversation

@romek-codes
Copy link

@romek-codes romek-codes commented May 19, 2023

#1882 directly related to this issue.

This is my first pull request to an open source project, any critique about things I could improve on will be appreciated 🥳

@mdmintz
Copy link
Member

mdmintz commented May 24, 2023

@RomanJus This is problematic, because with latest_logs/<very_long_node_id_from_a_test_item>/basic_test_info.txt, the part that gets cut off with your solution is from basic_test_info.txt, rather than from <very_long_node_id_from_a_test_item>. That filename should always be basic_test_info.txt for that file. It's the part with <very_long_node_id_from_a_test_item> that needs to be limited to 255 characters, which would need to be changed in a different file, not only to fix the max-file-length issue, but also to make sure that the Dashboard link to the specific latest_logs/ subfolder is correct.

A few other issues as well, such as using pass in a code block that already has other lines in it, making the pass useless.

And you have import errno, yet you never use errno directly, since you use os_error.errno in your except block.

@romek-codes
Copy link
Author

@RomanJus This is problematic, because with latest_logs/<very_long_node_id_from_a_test_item>/basic_test_info.txt, the part that gets cut off with your solution is from basic_test_info.txt, rather than from <very_long_node_id_from_a_test_item>. That filename should always be basic_test_info.txt for that file. It's the part with <very_long_node_id_from_a_test_item> that needs to be limited to 255 characters, which would need to be changed in a different file, not only to fix the max-file-length issue, but also to make sure that the Dashboard link to the specific latest_logs/ subfolder is correct.

A few other issues as well, such as using pass in a code block that already has other lines in it, making the pass useless.

And you have import errno, yet you never use errno directly, since you use os_error.errno in your except block.

Thanks for the critique, i'll modify it soon when i have some free time according to your tips 🤠

@mdmintz
Copy link
Member

mdmintz commented May 25, 2023

This was already resolved in 4.14.10 - https://github.com/seleniumbase/SeleniumBase/releases/tag/v4.14.10

@mdmintz mdmintz closed this May 25, 2023
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