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 TablePreferences entity #928

Merged
merged 2 commits into from
Jun 23, 2023

Conversation

lhellebr
Copy link
Contributor

@lhellebr lhellebr commented Jun 6, 2023

Description of changes

Added TablePreferences entity.

Functional demonstration

Shouldn't cause regression, this test uses search and hasn't changed:

$ pytest tests/foreman/api/test_contentview.py::test_positive_admin_user_actions
================================================================= test session starts =================================================================
platform linux -- Python 3.9.16, pytest-7.3.1, pluggy-1.0.0 -- /home/lhellebr/git/robottelo/venv/bin/python
Mandatory Requirements Available: jinja2==3.1.2 broker[docker]==0.3.2 pytest-xdist==3.3.1 wrapanapi==3.5.15 requests==2.31.0 pytest-reportportal==5.1.8
Optional Requirements Available: pre-commit==3.3.2 redis==4.5.5 manage>=0.1.13 sphinx==7.0.1
cachedir: .pytest_cache
shared_function enabled - OFF - scope:  - storage: file
rootdir: /home/lhellebr/git/robottelo
configfile: pytest.ini
plugins: cov-3.0.0, services-2.2.1, xdist-3.2.1, reportportal-5.1.7, mock-3.10.0, ibutsu-2.2.4
collected 1 item                                                                                                                                      

tests/foreman/api/test_contentview.py::test_positive_admin_user_actions PASSED                                                                  [100%]

================================================================= 1 passed in 56.59s ==================================================================

The same happens when I change that test to use ServerConfig explicitly, and don't change anything else (so it doesn't specify the new argument).

This is a new test using it:

$ pytest tests/foreman/api/test_user.py::TestUser::test_positive_table_preferences
================================================================= test session starts =================================================================
platform linux -- Python 3.9.16, pytest-7.3.1, pluggy-1.0.0 -- /home/lhellebr/git/robottelo/venv/bin/python
Mandatory Requirements Available: jinja2==3.1.2 pytest-xdist==3.3.1 broker[docker]==0.3.2 requests==2.31.0 wrapanapi==3.5.15 pytest-reportportal==5.1.8
Optional Requirements Available: redis==4.5.5 pre-commit==3.3.2 manage>=0.1.13 sphinx==7.0.1
cachedir: .pytest_cache
shared_function enabled - OFF - scope:  - storage: file
rootdir: /home/lhellebr/git/robottelo
configfile: pytest.ini
plugins: cov-3.0.0, services-2.2.1, xdist-3.2.1, reportportal-5.1.7, mock-3.10.0, ibutsu-2.2.4
collected 1 item                                                                                                                                      

tests/foreman/api/test_user.py::TestUser::test_positive_create_with_username 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB set_trace (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> /home/lhellebr/git/robottelo/tests/foreman/api/test_user.py(428)test_positive_create_with_username()
-> assert hasattr(tp, 'name')
(Pdb) c

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB continue (IO-capturing resumed) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
PASSED                                                             [100%]

================================================================= 1 passed in 58.13s ==================================================================

@lhellebr
Copy link
Contributor Author

lhellebr commented Jun 7, 2023

make docs-html fails and it also happens in master -> failure unrelated

@lhellebr
Copy link
Contributor Author

lhellebr commented Jun 8, 2023

The dependent robottelo PR has been merged. Can we also merge this?

Copy link
Member

@jyejare jyejare left a comment

Choose a reason for hiding this comment

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

The make docs failure is related to the change in the table-preferences entity only, please fix it !

Also, update unit tests for the the new class!

@lhellebr
Copy link
Contributor Author

Yes, this failure is a real one, I'll fix it. Previously, already the test invocation has been failing.

@codecov
Copy link

codecov bot commented Jun 12, 2023

Codecov Report

Patch coverage: 95.45% and project coverage change: +0.04 🎉

Comparison is base (cfed8c9) 92.18% compared to head (7b85ca2) 92.22%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #928      +/-   ##
==========================================
+ Coverage   92.18%   92.22%   +0.04%     
==========================================
  Files           6        6              
  Lines        3031     3050      +19     
==========================================
+ Hits         2794     2813      +19     
  Misses        237      237              
Impacted Files Coverage Δ
nailgun/entity_mixins.py 95.95% <66.66%> (ø)
nailgun/entities.py 91.28% <100.00%> (+0.06%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jyejare
Copy link
Member

jyejare commented Jun 12, 2023

Patch coverage is only 26% , so you should now add unit tests.

@lhellebr lhellebr force-pushed the table_preferences_entity branch 2 times, most recently from b7947cb to d4f7283 Compare June 12, 2023 14:02
@lhellebr
Copy link
Contributor Author

Yes, I was working on it, it just took a little while.
The report doesn't seem to be updated yet (let's wait a little bit, I don't know how to force refresh) but I think it should be ok now.

@lhellebr lhellebr requested a review from jyejare June 12, 2023 14:28
Copy link
Contributor

@sambible sambible left a comment

Choose a reason for hiding this comment

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

Any reason for not using PRT? Looks good either way. Also, you aren't alone on forgetting to add tests for Nailgun PRs :)

@lhellebr
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/api/test_contentview.py -k test_positive_admin_user_actions

@lhellebr lhellebr requested a review from a team June 15, 2023 11:36
@lhellebr
Copy link
Contributor Author

Who knows why I still don't see any PRT results. But I think we can merge. The test using this is merged already!

@omkarkhatavkar
Copy link
Contributor

Who knows why I still don't see any PRT results. But I think we can merge. The test using this is merged already!

PRT results get overridden once you add a new commit in the code.

@omkarkhatavkar
Copy link
Contributor

trigger: test-robottelo
pytest: tests/foreman/api/test_contentview.py -k test_positive_admin_user_actions

@omkarkhatavkar omkarkhatavkar merged commit 33db04b into SatelliteQE:master Jun 23, 2023
4 checks passed
github-actions bot pushed a commit that referenced this pull request Jun 23, 2023
* Add TablePreferences entity

* Tests for TablePreferences entity

(cherry picked from commit 33db04b)
ogajduse pushed a commit that referenced this pull request Jun 23, 2023
* Add TablePreferences entity

* Tests for TablePreferences entity

(cherry picked from commit 33db04b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.14.z CherryPick PR needs CherryPick to previous branches
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants