Skip to content
This repository has been archived by the owner on Apr 7, 2022. It is now read-only.

Developer Guidelines

Sean Myers edited this page Aug 28, 2013 · 15 revisions

General Guidelines

Contributing

  • Own your pull requests; you are their advocate.
    • If a request goes unreviewed for two or three days, ping a reviewer to see what's holding things up.
    • Follow up on open pull requests and respond to any comments or questions a reviewer might have.
  • Keep the contents of the pull request focused on one idea. Smaller pull requests are easier to review, and thus will be merged in more quickly.
  • After submitting a request, be ready to work closely with a reviewer to get it tested and integrated into the overall test suite.
  • Follow the Code Style guidelines to make your pull request as easy to review as possible.
  • If your request requires the use of private information that can't be represented in the data file templates (probably cfme_data.yaml), please state that in the test module docstring or the individual test docstring, along with information on where that data can be found.
  • Similar to the last point, any data files used by a test module should be clearly documented in that module's docstring.
  • Any data required in a sensitive data file should be reflected in the template for that file.
  • Standards may change over time, so copying older code with similar functionality may not be the most productive action. If in doubt, refer back to this document and update the copied code according to the current guidelines.

Reviewers

Reviewers will be looking to make sure that the Contributing guidelines are being met. Some of the things that go into the review process:

  • Newly added tests will be run against a clean appliance.
  • Adherence to code style guidelines will be checked.

If tests fail, reviewers WILL:

  • ...give you a complete traceback of the error.
  • ...give you useful information about the appliance against which tests were run, such as the appliance version.
  • ...give you insight into any related data files used.
  • ...not thoroughly debug the failing test(s).

Reviewers must never merge their own pull requests.

Code Style

We adhere to Python's PEP 8 style guide, occasionally allowing exceptions for the sake of readability. This is covered in the "Foolish Consistency" section of PEP 8.

We also do a few things that aren't explicitly called out in PEP 8:

  • When wrapping long conditionals, indent trailing lines twice, just like with function names and any other block statement (they usually end with colons):

      if this_extremely_long_variable_name_takes_up_the_whole_line and
              you_need_to_wrap_your_conditional_to_the_next_line:
          # Two indents help clearly separate the wrapped conditional
          # from the following code.
  • When indenting a wrapping sequence, one indent will do. Don't try to align all of the sequence items at an arbitrary column:

      a_good_list = [
          'item1',
          'item2',
          'item3'
      ]
      a_less_good_list = [ 'item1',
                           'item2',
                           'item3'
      ]
  • According to PEP 8, triple-quoted docstrings use double quotes. To help differentiate docstrings from normal multi-line strings, consider using single-quotes in the latter case:

      """This is a docstring.
    
      It follows PEP 8's docstring guidelines.
    
      """
      paragraph = '''This is a triple-quoted string, with newlines captured.
      PEP 8 and PEP 257 guidelines don't apply to this. Using single quotes here
      makes it simple for a reviewer to know that docstring style doesn't apply
      to this text block.'''
  • On the subject of docstrings (as well as comments) use them. Python is somewhat self-documenting, so use docstrings and comments as a way to explain not just what code is doing, but why it's doing what it is, and what it's intended to achieve.

  • In addition to being broken up into the three sections of standard library, third-party, and the local application, imports should be sorted alphabetically. 'import' lines within those sections still come before 'from ... import' lines:

      import sys
      from os import environ
      from random import choice

Other useful code style guidelines:

cfme_pages

pages are read-only python modeling of the CFME UI, used by cfme_tests

The cfme_pages project exists to model the CFME UI in Python, allowing the functional tests of the UI in cfme_tests to be ignorant of the underlying page structure. As such, UI elements (pages, regions, forms, etc.) modeled in cfme_pages must provide helper methods and properties to expose a usable interface to cfme_tests. This is explained in more detail in the section on "Writing Tests".

As mentioned in the cfme_pages README, pages should be modeled as a part of writing tests in cfme_tests. Code in cfme_pages must never depend on code in cfme_tests. If this is required, either the required code in cfme_tests needs to be moved to cfme_pages, or the code being written actually belongs in cfme_tests.

Layout

cfme_pages/

  • pages/ Top-level pages container. The structure of this directory should mimic the layout of the CFME UI as much as possible.
  • tests/ Top-level tests container.
  • common/ Common code that can be used outside of the test context. Modules here can be used by both cfme_pages and cfme_tests
  • plugin/ Common py.test plugins used by both cfme_pages and cfme_tests
  • scripts/ Useful scripts for QE developers that aren't used during a test run

Testing Pages

The pages tests are a cursory smoke test. Making them more thorough will likely end up duplicating effort between cfme_tests and cfme_pages.

Tests in cfme_pages have the following properties:

  • They pass on a freshly deployed appliance with no configuration beyond the defaults.
  • They are idempotent, producing the same result when run multiple times.
  • They make no changes to an appliance, leaving no evidence of having been run on an appliance.

It is not important for the pages tests to achieve 100% coverage on the pages themselves. cfme_pages is a vehicle through which we aim to achieve 100% coverage of the CFME UI in a cfme_tests test run. Therefore, 100% coverage of cfme_pages will only be achieved during the course of a complete cfme_tests test run.

Practically, this means that any test that enters data into the appliance belongs in cfme_tests. Pages that can't be tested without entering data into the appliance are acceptable, provided they are covered by code (or a related issue, but code is preferred) in cfme_tests.

cfme_tests

Read/write destructive testing of the CFME UI

Layout

cfme_tests/

  • tests/ Top-level tests container
    • appliance/ Appliance tests, generally using tools other than the UI to inspect appliance internals, like verify installed packages or required service states
    • scenario/ Large test scenarios, representing complicated setup and teardown workflows with interdependent tests
    • ui/ General UI tests, testing specific functional units of behavior
  • data/ Test data. The structure of this directory should match the structure under tests/, with data files for tests in the same relative location as the test itself.
    • For example, data files for tests/ui/test_ui_widgets.py could go into data/ui/test_ui_widgets/.
  • fixtures/ py.test fixtures that can be used by any test. Modules in this directory will be auto loaded.
  • markers/ py.test markers that can be used by any test. Modules in this directory will be auto loaded.
  • utils/ Utility functions that can be called inside our outside the test context. Generally, util functions benefit from having a related test fixture that exposes the utility to the tests. Modules in this directory will be auto loaded.
  • db/ The 'db' module provides access to an appliance's database, as well as convenience mappings for model to table names.
  • scripts/ Useful scripts for QE developers that aren't used during a test run

Writing Tests

Tests in cfme_tests have the following properties:

  • They pass on a freshly deployed appliance with no configuration beyond the defaults (i.e. tests do their own setup and teardown).
  • They never directly access a page's 'testsetup' attribute, or call selenium methods. Instead, methods defined on the page objects will carry out the required behavior.
  • Where possible, they strive to be idempotent to facilitate repeated testing and debugging of failing tests. (Repeatable is Reportable)
  • Where possible, they try to clean up behind themselves. This not only helps with idempotency, but testing all of the CRUD interactions helps to make a thorough test.
  • Tests should be thoroughly distrustful of the appliance, and measure an action's success in as many ways as possible. A practical example:
    • Do not trust flash messages, as they sometimes tell lies (or at least appear to). If you can go beyond a flash message to verify a test action, do so.

Fixtures

Fixtures are not only responsible for setting up tests, but also cleaning up after a test run, whether that test run succeeded or failed. addfinalizer is very powerful. finalizer functions are called even if tests fail.

When writing fixtures, consider how useful they might be for the overall project, and place them accordingly. Putting fixtures into a test module is rarely the best solution. Instead, try to put them in the nearest conftest.py. If they're generic/useful enough consider putting them into one of the fixtures/ directory for use in cfme_tests or the plugin/ directory for use in both projects.

This Document

This page is subject to change as our needs and policies evolve. Suggestions are always welcome.

Coming Soon

There are a lot of generally useful fixtures, marks, and utilities that should probably be mentioned in here. They'll be appearing soon.

Clone this wiki locally