Skip to content

SubmissionChecklist

Lucas Meneghel Rodrigues edited this page Sep 22, 2011 · 6 revisions

Autotest Code Submission Check List

This document describes to contributors what we are looking for when we go through submitted patches. Please try to follow this as much as possible to save both the person reviewing your code as well as yourself some extra time.

Running Unit tests

Regardless of what you change it is recommended that you not only add unittests but also run the unittest_suite.py in the AUTOTEST_ROOT to be sure any changes you made did not break anything. Example:

johndoe@bibbybob [~/src/autotest-devel]$./unittest_suite.py
running test /home/johndoe/src/autotest-devel/client/bin/job_unittest.py
running test /home/johndoe/src/autotest-devel/client/bin/kernel_versions_unittest.py
running test /home/johndoe/src/autotest-devel/client/bin/harness_unittest.py
running test /home/johndoe/src/autotest-devel/client/common_lib/site_containers_unittest.py
running test /home/johndoe/src/autotest-devel/client/common_lib/global_config_unittest.py
running test /home/johndoe/src/autotest-devel/client/common_lib/control_data_unittest.py
running test /home/johndoe/src/autotest-devel/client/common_lib/mail_unittest.py
running test /home/johndoe/src/autotest-devel/client/common_lib/barrier_unittest.py
running test /home/johndoe/src/autotest-devel/client/common_lib/site_platform_detector_unittest.py
running test /home/johndoe/src/autotest-devel/frontend/frontend_unittest.py
running test /home/johndoe/src/autotest-devel/scheduler/monitor_db_unittest.py
running test /home/johndoe/src/autotest-devel/new_cli/action_common_unittest.py
running test /home/johndoe/src/autotest-devel/new_cli/atest_unittest.py
running test /home/johndoe/src/autotest-devel/new_cli/host_unittest.py
running test /home/johndoe/src/autotest-devel/new_cli/user_unittest.py
running test /home/johndoe/src/autotest-devel/new_cli/label_unittest.py
running test /home/johndoe/src/autotest-devel/new_cli/acl_unittest.py
running test /home/johndoe/src/autotest-devel/new_cli/topic_common_unittest.py
running test /home/johndoe/src/autotest-devel/migrate/migrate_unittest.py
running test /home/johndoe/src/autotest-devel/server/utils_unittest.py
running test /home/johndoe/src/autotest-devel/server/container_unittest.py
running test /home/johndoe/src/autotest-devel/server/autotest_unittest.py
running test /home/johndoe/src/autotest-devel/server/base_server_job_unittest.py
running test /home/johndoe/src/autotest-devel/server/hosts/bootloader_unittest.py
running test /home/johndoe/src/autotest-devel/server/hosts/base_classes_unittest.py
running test /home/johndoe/src/autotest-devel/server/hosts/remote_unittest.py
running test /home/johndoe/src/autotest-devel/server/hosts/site_host_unittest.py
running test /home/johndoe/src/autotest-devel/server/site_common/cluster_common_unittest.py
running test /home/johndoe/src/autotest-devel/tko/status_lib_unittest.py
running test /home/johndoe/src/autotest-devel/tko/utils_unittest.py
running test /home/johndoe/src/autotest-devel/tko/parsers/version_0_unittest.py
running test /home/johndoe/src/autotest-devel/tko/parsers/version_1_unittest.py
running test /home/johndoe/src/autotest-devel/tcpserver/jfe_log_retrieval_unittest.py
running test /home/johndoe/src/autotest-devel/tcpserver/kernel_uploader_unittest.py
running test /home/johndoe/src/autotest-devel/tcpserver/tcpcommon_unittest.py
All tests passed!

Running pylint

Another tool we use to insure the correctness of our code is pylint. Due to the way imports have been implemented in the autotest code base a special wrapper is required to run pylint.

The file is located in $AUTOTEST_ROOT/utils/run_pylint.py

Simply run the command from your code directory and the rest is taken care of.

Example of running on disktest with errors:

************* Module disktest

The lines in bold are actual errors that need to be addressed where as the italic lines are false positives.

Breaking up changes

  • Submit a separate CL for each logical change (if your description includes "add this, fix that, remove three other unrelated things"; probably bad)
  • Put cleanups in a separate CL from functional changes.

CL Descriptions

Change list descriptions need to be as verbose as possible. Some of these points are obvious but still worth mentioning. * Describe:

  • The motivation for the change - what problem are you trying to fix?
  • High level description / design approach of how your change works (for non-trivial changes)
  • Implementation details
  • Testing results
  • Any concerns, or future things to fix.
  • Include Risk factor if possible (e.g. If you add more than a few lines to the scheduler risk is high)
    • Anything Affecting the whole system is considered high risk (e.g. autoserv, scheduler, or common_lib changes)
    • Web frontend changes are considered low risk
    • Test Changes are considered low risk
  • Visibility: Include whether or not the change is visible to users or not
  • Include any relevant bugs

Follow control file specification

All tests must follow the control file specification Refer to: http://test.kernel.org/autotest/ControlRequirements

Follow Coding Style

Autotest has its own unique coding style, please familiarize yourself with the following document: Refer to: http://test.kernel.org/trac-autotest/browser/trunk/CODING_STYLE

Example CL

Test's format_result() scans stdout before it has been flushed, losing some or all keyvals.
Six tests create their keyval files by scanning the debugdir/stdout disk file created by job.run_test().
This scanning was done without any flushing of pending stdout writes, when the disk file is shorter
than its logical size.  This could lead to loss of some or all keyvals, depending on whether the system
is also busy with other work at this moment.

This change was tested via twoway-container testing of simultaneous dbench and tbench jobs, and ordinary
non-container testing of bonnie.  The revised btreplay, reaim, and sysbench tests could not be tested,
because their prior version was not runnable.

Risk: Low (Only tests are modified)
Visibility: Users who use bonnie, btreplay, dbench, reaim, sysbench and tbench may notice a change
Bugs: http://test.kernel.org/trac-autotest/ticket/2
Clone this wiki locally