Skip to content

Conversation

@ramir-dn
Copy link
Contributor

@ramir-dn ramir-dn commented Nov 20, 2024

Hellow,

This PR handles 2 things:

  1. Fixes an issue with rp_dir_level: in _remove_root_dirs, the loop that handles the ROOT node only handled it first child and called return.
  2. Add back the support for rp_display_suite_test_file. It depends on rp_dir_level in a way that if the file name appears deep in the hierarchy, it will be kept and not deleted. The default is True to keep the current behavior, but if set to False, and the rp_dir_level is deep enough, the file name will be removed too, and will be left with suites and tests only in the hierarchy.

@ramir-dn ramir-dn changed the title Reneable rp_display_suite_test_file and fix bug with rp_dir_level Restore rp_display_suite_test_file and fix bug with rp_dir_level Nov 20, 2024
@ramir-dn ramir-dn marked this pull request as ready for review November 21, 2024 13:36
@ramir-dn
Copy link
Contributor Author

Hello @HardNorth , will be great if you could take a look and review this. If that's fine, it will be great to have a new release with this change. thanks!

@ramir-dn
Copy link
Contributor Author

ramir-dn commented Dec 2, 2024

Hi @HardNorth, anything I can do to promote this PR? thanks you!

Copy link
Member

@HardNorth HardNorth left a comment

Choose a reason for hiding this comment

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

@ramir-dn, Thanks for the suggestion, but looks like you checked out broken code which was in develop branch for one day. Please pull the latest changes.

@ramir-dn
Copy link
Contributor Author

ramir-dn commented Dec 2, 2024

Thanks @HardNorth , updated my branch. Can you test again please?

@ramir-dn
Copy link
Contributor Author

ramir-dn commented Dec 2, 2024

@HardNorth , I fixed the missing parameter. I am not sure what happened in my local tox run but it didn't detect it. Can you please rerun?

@codecov-commenter
Copy link

codecov-commenter commented Dec 2, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 68.22%. Comparing base (1bcb242) to head (7b6cec8).
Report is 17 commits behind head on develop.

Files with missing lines Patch % Lines
pytest_reportportal/service.py 81.48% 5 Missing ⚠️
pytest_reportportal/config.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #383      +/-   ##
===========================================
- Coverage    68.35%   68.22%   -0.13%     
===========================================
  Files            6        6              
  Lines          970      982      +12     
===========================================
+ Hits           663      670       +7     
- Misses         307      312       +5     
Flag Coverage Δ
unittests 68.22% <80.00%> (-0.13%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@HardNorth HardNorth left a comment

Choose a reason for hiding this comment

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

LGTM

@HardNorth HardNorth merged commit ea3e87c into reportportal:develop Dec 2, 2024
7 checks passed
@ramir-dn
Copy link
Contributor Author

ramir-dn commented Dec 3, 2024

Thanks @HardNorth!
Can you tell when can I expect a new release with this fix?

@ramir-dn ramir-dn deleted the restore_rp_display_suite_test_file branch December 3, 2024 04:32
@HardNorth
Copy link
Member

@ramir-dn Please wait for a day or two. I have to rework your code and add tests.

@HardNorth
Copy link
Member

HardNorth commented Dec 3, 2024

@ramir-dn Please use this version: https://github.com/reportportal/agent-python-pytest/releases/tag/5.4.6
I renamed your flag to rp_hierarchy_test_file, to conform with other flags, and removed any chain with rp_hierarchy_dirs_level flag, so you can use it independently.

@ramir-dn
Copy link
Contributor Author

ramir-dn commented Dec 3, 2024

That was quick! Thanks @HardNorth!

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.

3 participants