Skip to content

Commit

Permalink
Merge branch 'master' into no-blank-cvpage
Browse files Browse the repository at this point in the history
  • Loading branch information
sambible authored Oct 3, 2023
2 parents 47538ee + a07594f commit ded54a6
Show file tree
Hide file tree
Showing 372 changed files with 4,770 additions and 5,377 deletions.
2 changes: 0 additions & 2 deletions .flake8

This file was deleted.

2 changes: 0 additions & 2 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ updates:
- "6.14.z"
- "6.13.z"
- "6.12.z"
- "6.11.z"

# Maintain dependencies for our GitHub Actions
- package-ecosystem: "github-actions"
Expand All @@ -28,4 +27,3 @@ updates:
- "6.14.z"
- "6.13.z"
- "6.12.z"
- "6.11.z"
16 changes: 15 additions & 1 deletion .github/workflows/auto_cherry_pick.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ env:
assignee: ${{ github.event.pull_request.assignee.login }}
title: ${{ github.event.pull_request.title }}
number: ${{ github.event.number }}
is_dependabot_pr: ''

jobs:

Expand Down Expand Up @@ -40,12 +41,24 @@ jobs:
label: ${{ github.event.pull_request.labels.*.name }}

steps:
# Needed to avoid out-of-memory error
- name: Set Swap Space
uses: pierotofy/set-swap-space@master
with:
swap-size-gb: 10

## Robottelo Repo Checkout
- uses: actions/checkout@v3
- uses: actions/checkout@v4
if: ${{ startsWith(matrix.label, '6.') && matrix.label != github.base_ref }}
with:
fetch-depth: 0

## Set env var for dependencies label PR
- name: Set env var is_dependabot_pr to `dependencies` to set the label
if: contains(github.event.pull_request.labels.*.name, 'dependencies')
run: |
echo "is_dependabot_pr=dependencies" >> $GITHUB_ENV
## CherryPicking and AutoMerging
- name: Cherrypicking to zStream branch
id: cherrypick
Expand All @@ -58,6 +71,7 @@ jobs:
Auto_Cherry_Picked
${{ matrix.label }}
No-CherryPick
${{ env.is_dependabot_pr }}
assignees: ${{ env.assignee }}

- name: Add Parent PR's PRT comment to Auto_Cherry_Picked PR's
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/dependency_merge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
github-token: "${{ secrets.GITHUB_TOKEN }}"

- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4
with:
fetch-depth: 0

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/dispatch_release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4

- name: Git User setup
run: "git config --local user.email Satellite-QE.satqe.com && git config --local user.name Satellite-QE"
Expand Down
11 changes: 9 additions & 2 deletions .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
python-version: ['3.10', '3.11']
steps:
- name: Checkout Robottelo
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Set Up Python-${{ matrix.python-version }}
uses: actions/setup-python@v4
Expand Down Expand Up @@ -46,18 +46,25 @@ jobs:

- name: Collect Tests
run: |
# To skip vault login in pull request checks
export VAULT_SECRET_ID_FOR_DYNACONF=somesecret
pytest --collect-only --disable-pytest-warnings tests/foreman/ tests/robottelo/
pytest --collect-only --disable-pytest-warnings -m pre_upgrade tests/upgrades/
pytest --collect-only --disable-pytest-warnings -m post_upgrade tests/upgrades/
- name: Collect Tests with xdist
run: |
# To skip vault login in pull request checks
export VAULT_SECRET_ID_FOR_DYNACONF=somesecret
pytest --collect-only --setup-plan --disable-pytest-warnings -n 2 tests/foreman/ tests/robottelo/
pytest --collect-only --setup-plan --disable-pytest-warnings -n 2 -m pre_upgrade tests/upgrades/
pytest --collect-only --setup-plan --disable-pytest-warnings -n 2 -m post_upgrade tests/upgrades/
- name: Run Robottelo's Tests
run: pytest -sv tests/robottelo/
run: |
# To skip vault login in pull request checks
export VAULT_SECRET_ID_FOR_DYNACONF=somesecret
pytest -sv tests/robottelo/
- name: Make Docs
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/update_robottelo_image.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:

steps:
- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Get image tag
id: image_tag
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/weekly.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: [3.9]
python-version: [3.11]
steps:
- name: Checkout Robottelo
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Set Up Python-${{ matrix.python-version }}
uses: actions/setup-python@v4
Expand Down
17 changes: 4 additions & 13 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,31 +1,22 @@
# configuration for pre-commit git hooks

repos:
- repo: https://github.com/asottile/reorder_python_imports
rev: v3.9.0
hooks:
- id: reorder-python-imports
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
hooks:
- id: trailing-whitespace
exclude: tests/foreman/data/
- id: end-of-file-fixer
- id: check-yaml
- id: debug-statements
- repo: https://github.com/asottile/pyupgrade
rev: v3.3.0
hooks:
- id: pyupgrade
args: [--py38-plus]
- repo: https://github.com/psf/black
rev: 22.10.0
hooks:
- id: black
- repo: https://github.com/pycqa/flake8
rev: 6.0.0
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.0.277
hooks:
- id: flake8
- id: ruff
args: [--fix, --exit-non-zero-on-fix]
- repo: local
hooks:
- id: fix-uuids
Expand Down
149 changes: 149 additions & 0 deletions BEST_PRACTICES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
# Helpers Best Practices - Guide
Welcome to the **Helpers Best Practices Guide**! This comprehensive guide is designed to assist contributors in locating existing helpers and adding new ones to the right place, making them accessible through appropriate objects, fixtures, and importables. By following these best practices, we aim to improve the organization of helpers within the framework, enhance the discovery of helpful tools, and minimize the duplication of helper definitions and maintenance efforts.

## Introduction
The scattered and misaligned placement of helpers within the framework can lead to challenges in discovering and managing helpers, resulting in duplicated definitions and increased maintenance overhead. To address these issues, we have outlined best practices for defining helper functions, modules, classes and fixtures. These guidelines will help you make informed decisions about where to place different types of helpers within the framework.


## General Practices
Here are some general best practices for helper management:
- **Prefer API Calls**: When creating test setups, prioritize using API calls (nailgun methods) over UI and CLI interactions, as they tend to be faster and more efficient, unless there is a specific need for specific endpoint based setups. UI setups should be the last resort.
- **Utilize `target_sat` Fixtures**: Ensure that all tests make use of target satellite fixtures to access a wide range of helper methods provided by the satellite object.
- **Non-Reusable helpers**: If helper/fixture is not reusable, keep them closer to test in test module itself.
- **No one liner functions**: If the helper function is just a one-liner, It's recommended to directly replace the function code where it's used.

## Python Packages and Modules:

### pytest_fixtures

In the `pytest_fixtures` Python package, we provide globally accessible fixtures for use in all tests and fixtures. These fixtures are cached by scopes, making them highly reusable without the need for reimplementation. There are two types of fixtures within this package:

#### Components
- Component fixtures serve as setup and teardown utilities for component tests.
- Each individual fixture module in this package contains and represents fixtures for specific components.

#### Core
- Core fixtures are framework-level fixtures that serve as setup and teardown for component tests.
- Several special fixture modules deserve mention here:
- `broker.py`: Contains all the `target_sat` fixtures, scoped by usage, which should be used in every functional test in `robottelo`. These fixtures return the same satellite instance for all scoped fixtures, provided that the calling test does not have a destructive marker. For tests with destructive markers, they return a new satellite instance from an image.
- `contenthosts.py`: Houses all content host creation fixtures. All fixture markers for content hosts should be added here since the `fixture_markers` pytest plugin operates on these fixtures, adding markers and user properties to tests that use them.
- `sat_cap_factory.py`: Contains fixtures that create satellites/capsules from scratch without using the xDist satellite. These fixtures should be placed here, as the `factory_collection` pytest plugin operates on them, adding markers to tests that use these fixtures.
- `xdist.py`: Includes the xDist distribution session-scoped fixture, which distributes and generates satellites for running Robottelo tests. The behavior of xDist is based on the `xdist_behavior` set in the `server.yaml` configuration file.

### Robottelo

#### host_helpers
- `api_factory.py`
- This module contains the `APIFactory` class. Helpers within this class facilitate integrated operations between components of the satellite object using satellite API calls.
- Since we favor API test setups, this class is the preferred location for adding generic helpers that can be used across UI, CLI, and API tests.
- Helpers added to this class or inherited by this class can be accessed as `sat_obj.api_factory.helper_func_name()`.

- `cli_factory.py`
- This module contains the `CLIFactory` class. Helpers within this class enable integrated operations between components of the satellite object using satellite CLI hammer commands.
- While it is not our primary choice to add more CLI-based helpers, this class is the appropriate place to add them if the test demands it.
- Helpers added to this class or inherited by this class can be accessed as `sat_obj.cli_factory.helper_func_name()`.

- `ui_factory.py`
- This module contains a `UIFactory` class that accepts a session object as a parameter. Helpers within this class enable integrated operations between components of the satellite object using the satellite UI.
- Although adding more UI-based helpers is not our preference, this class is suitable for adding them if the test demands it.
- Helpers added to this class or inherited by this class can be accessed as `sat_obj.ui_factory(session).helper_func_name()`.
- The session object provided here is the existing session object from the test case where the helper is being accessed.
- One can use different session object when session is created for different user parameters if needed.

- `*_mixins.py`
- Classes in mixin modules are inherited by the `Satellite`, `Capsule`, and `Host` classes in the `robottelo/hosts.py` module. Refer to the `hosts.py` module section for more details on that class.
- Mixin classes in these mixin modules and their methods extend functionalities for the host classes in `robottelo/hosts.py` mentioned on above line.
- Unlike methods in `robottelo/hosts.py`, these methods do not pertain to the hosts themselves but perform operations on those hosts and integration between host components.

#### utils
- Helpers/Utilities which are not directly related to any host object (that does not need operations on hosts) should be added to utils package.
- We have special utility modules for a specific subject. One can add utility helpers in those modules if the helper relates to any of the utlity modules or create new module for new specific subject.
- If the helper does not fit into any of the existing utils modules or subject of helper(s) is not big enough to create new , then add in `robottelo/utils/__init__.py` as a standalone helper.

#### hosts.py
The `hosts.py` module primarily contains three main classes that represent RedHat Satellite's hosts. Each higher ordered class here inherits the behavior of the previous host class.:

- **ContentHost**:
- This class represents the ContentHost or Client, including crucial properties and methods that describe these hosts.
- Properties like `hostname` specify details about specific ContentHosts.
- The `nailgun_host` property returns the API/nailgun object for the ContentHost.
- Methods within this class manage operations on ContentHosts, particularly for reconfiguring them. Other operational methods are part of the ContentHost mixins.
- This class inherits methods from `Brokers` modules `Host` class, hence methods from that class are available with this class objects.
- **Capsule**:
- This class represents the Capsule host and includes essential properties and methods that describe the Capsule host.
- Properties like `url` and `is_stream` specify details about the specific Capsule host.
- The `satellite` property returns the satellite object to which the Capsule is connected.
- Methods within this class manage operations related to the Capsule, especially for reconfiguring it via installation processes. Other operational methods are part of the Capsule mixins. e.g `sat.restart_services()`
- This class inherits methods from ContentHost class, hence methods from these classes are available with this class objects.
- **Satellite**:
- This class serves as the representation of the Satellite host and includes essential properties and methods that describe the Satellite host.
- Properties like `hostname` and `port` specify specific details about the Satellite host.
- Properties like `api`, `cli`, and `ui_session` provide access to interface objects that expose satellite components through various endpoints. For example, you can access these components as `sat.api.ActivationKey`, `sat.cli.ActivationKey`, or `sat.ui_session.ActivationKey`.
- Methods within this class handle satellite operations, especially for reconfiguring the satellite via installation processes etc. Other operational methods are part of the Satellite mixins, including factories. e.g `sat.capsule_certs_generate()`
- This class inherits methods from ContentHost and Capsule classes, hence methods from those classes are available with this class objects.

By adhering to these best practices, you'll contribute to the efficient organization and accessibility of helpers within the framework, enhancing the overall development experience and maintainability
of the project. Thank you for your commitment to best practices!


## FAQs:

_**Question. When should I prefer/not prefer fixtures over to helper function when implementing a new helper ?**_

_Answer:_ The answers could be multiple. Fixture over functions are preferred when:
- A new helper function should be cached based on scope of the function usage.
- A new helper has dependency on other fixtures.
- A new helper provides the facility of setup and teardown in the helper function itself.
- A new helper as a fixture accept parameters, allowing you to create dynamic test scenarios.

_**Question. Where should I implement the fixture that could be used by all tests from all endpoints?**_

_Answer:_ The global package `pytest_fixtures` is the recommended place. Choose the right subpackage `core` if framework fixture `component` if component fixture.

_**Question. Where should we implement the helper functions needed by fixtures ?**_

_Answer:_ It is preferred to use reusable helper function in utils modules but if the function is not reusable, it should live in the fixture own module.

_**Question. Whats the most preferred user interface for writing helper that could be used across all three interfaces tests ?**_

_Answer:_ API helpers are preferred but if test demands setup from UI/CLI interfaces then the helper should be added in those interfaces.

_**Question. I want to create a property / method to describe satellite/capsule/host, where should I add that property ?**_

_Answer:_ Satellite/Caspule/ContentHost classes (but not in their mixins) are the best to add properties/methods that describe those.

_**Question. I want to perform operations on Satellite/Capsule/ContentHost like execute CLI commands, read conf files etc, where should I add a method for that?**_

_Answer:_ Add into existing mixin classes in mixin modules in `robottelo/host_helpers/*_mixins.py` or create a new targeted mixin class and inherit than in host classes of `robottelo/hosts.py` module

_**Question. I have new CLI or UI or API entity or operation to implement, should it be in factory object or endpoint object.**_

_Answer:_ New entity/subcommand/operations added in satellite product should be added to be accessed by endpoint onjects. e.g `target_sat.api.ComponentNew()`. The factory objects are just for adding helpers.

_**Question. When should I prefer helper function as a host object method over utils function ?**_

_Answer:_ When the new function is dependent on existing host methods or attributes then its good implement such helper function in host classes as it makes to access those host methods and attributes easier.

_**Question. What helper functions goes into `robottelo/utils/__init__.py` module?**_

_Answer:_ If the helper does not fit into any of the existing utils modules or subject of helper(s) is not big enough to create new , then add in `robottelo/utils/__init__.py` as a standalone helper.

_**Question. I have a helper function that applies to all Satellite, Capsule, ContentHost classes. Which class should I choose to add it?**_

_Answer:_ The recommeded class in such cases is always the highest parent class. Here ContentHost class is the highest parent class where this function should be added as a class method or a mixin method.

_**Question. I have some upgrade scenario helpers to implement, where should I add them?**_

_Answer:_ Upgrade scenarios are not special in this case. All helpers/function except framework should be same as being used in foreman tests and hence all rules are applied.

_**Question. Where should a non-reusable helper function reside?**_

_Answer:_ The preffered place is the test module where its being used.

_**Question. I need to extend the functionality of third party library methods/objects, should I do it in robottelo?**_

_Answer:_ Its recommended to extend the functionality of third library methods in that library itself if its being maintained by SatelliteQE like airgun,nailgun, manifester or is active community like widgetastic, wrapanapi. Else extend that appropriately in utils, host methods or fixtures.

_**Question. What if I see two helper methods/functions almost doing equal operations with a little diff ?**_

_Answer:_ See if this could be merged in one function with optional parameters else leave them separated. E.g provisioning functions using API calls with minimum difference and being used in API, UI and CLI tests then merge them. But if two provisioning helpers one for CLI and API tests and tests demands it then good to keep it separate.
1 change: 1 addition & 0 deletions conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

pytest_plugins = [
# Plugins
'pytest_plugins.auto_vault',
'pytest_plugins.disable_rp_params',
'pytest_plugins.external_logging',
'pytest_plugins.fixture_markers',
Expand Down
32 changes: 32 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,38 @@ exclude = '''
)/
'''

[tool.ruff]
target-version = "py311"
fixable = ["ALL"]

select = [
# "C90", # mccabe
"E", # pycodestyle
"F", # flake8
"I", # isort
# "Q", # flake8-quotes
"UP", # pyupgrade
"W", # pycodestyle
]

ignore = [
"E501", # line too long - handled by black
]

[tool.ruff.isort]
force-sort-within-sections = true
known-first-party = [
"robottelo",
]
combine-as-imports = true


[tool.ruff.flake8-quotes]
inline-quotes = "single"

[tool.ruff.mccabe]
max-complexity = 20

[tool.pytest.ini_options]
junit_logging = 'all'
addopts = '--show-capture=no'
3 changes: 1 addition & 2 deletions pytest_fixtures/component/acs.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
# Alternate Content Sources fixtures
import pytest

from robottelo.constants.repos import CUSTOM_FILE_REPO
from robottelo.constants.repos import CUSTOM_RPM_REPO
from robottelo.constants.repos import CUSTOM_FILE_REPO, CUSTOM_RPM_REPO


@pytest.fixture(scope='module')
Expand Down
2 changes: 1 addition & 1 deletion pytest_fixtures/component/computeprofile.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Compute Profile Fixtures
import pytest
from nailgun import entities
import pytest


@pytest.fixture(scope='module')
Expand Down
2 changes: 1 addition & 1 deletion pytest_fixtures/component/contentview.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Content View Fixtures
import pytest
from nailgun.entity_mixins import call_entity_method_with_timeout
import pytest

from robottelo.constants import DEFAULT_CV

Expand Down
Loading

0 comments on commit ded54a6

Please sign in to comment.