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

Added Infra view, conf values, add provider test #184

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

Conversation

vprusa
Copy link
Contributor

@vprusa vprusa commented Aug 21, 2017

The view/infrastructure.py is edited copy of views/providers.py
There is a lot of same code for all providers and some variant strings (url, provider name, check messages).
Imo there could be used inheritance for all providers, but that would mean refactoring provdiers.py and possibly all its usages so i did not touch that.
My steps for changing it would be:

  • Change providers as base class for all other providers called providers.py that would extend to middleware, openshift, infrastructure classes that would contain same/similar methods with different URL parts.
  • Change all usages of import for providers.py to middleware.py

The coverage of polarion testcases is still missing and im working on them (due to issues with wrong and unavailable hostname for RHEVM im waiting for response from mpusateri on how to handle that).

I would also change the common/db.py so there would not be the check on lines 61-66 and the method would consume provider type.

The INFRA_TEST_VM_NAME param in conf/properties.properties is for INV008 testcases, all of them run on 1 VM and may be many other test VMs (not just ours).

@mattmahoneyrh
Copy link
Contributor

The change to db.py is good. Perhaps move that chance into another branch and then generate a PR just for that change. This way this fix is not blocked waiting the infrastructure class to be reviewed and tested.

@vprusa vprusa mentioned this pull request Aug 25, 2017
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