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

[WIP] Add Tier validation #143

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mirekdlugosz
Copy link
Contributor

In Satellite we have one special metadata field - tier. It is special, because it is defined as function decorator instead of token in docstring. It is also special because it is synthetic variable that mostly depends on values of other variables.

This PR introduces new command that checks if assigned tier is in line with algorithm of creating tiers. It's in no shape to be merged yet (and definitely not before #142), but I want to start conversation.

@elyezer do you think something like that would fit into testimony?
I believe that tier computation rules are too complex to fit into config file. On the other hand, this is extremely specific to Satellite (Robottelo) and I don't see it being very usable for anyone else. Then, I'm not sure if anyone outside Satellite uses testimony.

I could imagine this being a Satellite-specific plugin, but... we don't have any infrastructure for plugins in testimony. And honestly, I don't have time to work on adding plugins support - I would rather have my peers use private fork / special branch.

@mirekdlugosz mirekdlugosz requested a review from elyezer May 23, 2019 13:28
@sghai
Copy link

sghai commented May 24, 2019

I liked this change.. looks like it ensure the correct tier being set on tests..

@elyezer
Copy link
Collaborator

elyezer commented May 24, 2019

Testimony is meant to be a generic tool that allows users to customize it to match their own tokens and validations. These changes are very team specific and therefore does not match the purpose of the project.

I agree that adding plugin support is a long term feature and won't be something that we will have soon. That said I would encourage create a fork until we have the ability to customize and add extra validation commands.

In the mean time let's open an issue and start designing the support to custom validation and/or creating hooks that Testimony will call to do extra job.

Copy link

@pgagne pgagne left a comment

Choose a reason for hiding this comment

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

@Mirzal why do you have functions like _get_tier and validate_tiers in __init__.py they don't belong here.

@mirekdlugosz
Copy link
Contributor Author

@pgagne
The main purpose of this PR was to start conversation if this feature is something that we can include in testimony.

Then, we are in the middle of test metadata cleanup and have deadline for this task set already. Having any kind of tool that could help me with tier assignment was priority for me. Enabling others to use this tool was quite high on priority list, too. Refactoring stable tool to conform to Python project layout best practices is not in my list of top 10 things to do.

I agree with you on principle. Using __init__.py for main program logic and general storage for any related class is one of many architectural flaws in testimony. Having worked with the code recently, I can tell that tool could use some major refactoring. If you are up to this task, I will happily discuss, share ideas and review code changes.

@elyezer
Copy link
Collaborator

elyezer commented May 29, 2019

Testimony used to be a simple tool that evolved and the architecture was not updated completely. Basically what was done initially on the __init__.py stayed there, new code was properly moved to other modules.

Any improvement on the architecture will be welcome for sure.

@Mirzal you've mentioned about Python project best practices, do you have a link?

@mirekdlugosz
Copy link
Contributor Author

@elyezer I actually meant https://docs.python.org/3/tutorial/modules.html , which Perry linked to in another thread.

For "best practices", I would probably use Python guide, or RealPython (+ this) as reference. But we should remember that "best practices" are always opinionated and every team has their own.

@sthirugn
Copy link
Contributor

@mirekdlugosz If we can externalize the tier definitions, this is a good improvement for testimony.

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.

5 participants