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.

Sign up for GitHub

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

Jump to bottom

Add new options to hacking/shippable/incidental.py #68384

Merged
merged 3 commits into from Mar 20, 2020
Merged

Add new options to hacking/shippable/incidental.py #68384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
909af25
Add `--plugin-path` option to `incidental.py`.
mattclay Mar 20, 2020
24103fb
Report on plugins with no test target.
mattclay Mar 20, 2020
617df4c
Add `--verbose` option to script.
mattclay Mar 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions hacking/shippable/README.md
View file
Open in desktop
Expand Up @@ -62,6 +62,26 @@ When incidental tests no longer provide exclusive coverage they can be removed.

> CAUTION: Only one incidental test should be removed at a time, as doing so may cause another test to gain exclusive incidental coverage.

#### Incidental Plugin Coverage

Incidental test coverage is not limited to ``incidental_`` prefixed tests.
For example, incomplete code coverage from a filter plugin's own tests may be covered by an unrelated test.
The ``incidental.py`` script can be used to identify these gaps as well.

Follow the steps 1 and 2 as outlined in the previous section.
For step 3, add the ``--plugin-path {path_to_plugin}`` option.
Repeat step 3 for as many plugins as desired.

To report on multiple plugins at once, such as all ``filter`` plugins, the following command can be used:

```shell
find lib/ansible/plugins/filter -name '*.py' -not -name __init__.py -exec hacking/shippable/incidental.py ansible/ansible/162160 --plugin-path '{}' ';'
```

Each report will show the incidental code coverage missing from the plugin's own tests.

> NOTE: The report does not identify where the incidental coverage comes from.

### Reading Incidental Coverage Reports

Each line of code covered will be included in a report.
Expand Down
149 changes: 112 additions & 37 deletions hacking/shippable/incidental.py
View file
Open in desktop
Expand Up @@ -68,11 +68,6 @@ def parse_args():
default=source,
help='path to git repository containing Ansible source')

parser.add_argument('--targets',
type=regex,
default='^incidental_',
help='regex for targets to analyze, default: %(default)s')

parser.add_argument('--skip-checks',
action='store_true',
help='skip integrity checks, use only for debugging')
Expand All @@ -82,6 +77,20 @@ def parse_args():
action='store_false',
help='ignore cached files')

parser.add_argument('-v', '--verbose',
action='store_true',
help='increase verbosity')

targets = parser.add_mutually_exclusive_group()

targets.add_argument('--targets',
type=regex,
default='^incidental_',
help='regex for targets to analyze, default: %(default)s')

targets.add_argument('--plugin-path',
help='path to plugin to report incidental coverage on')

if argcomplete:
argcomplete.autocomplete(parser)

Expand Down Expand Up @@ -149,18 +158,39 @@ def incidental_report(args):

# combine coverage results into a single file
combined_path = os.path.join(output_path, 'combined.json')
cached(combined_path, args.use_cache,
cached(combined_path, args.use_cache, args.verbose,
lambda: ct.combine(coverage_data.paths, combined_path))

with open(combined_path) as combined_file:
combined = json.load(combined_file)

if args.plugin_path:
# reporting on coverage missing from the test target for the specified plugin
# the report will be on a single target
cache_path_format = '%s' + '-for-%s' % os.path.splitext(os.path.basename(args.plugin_path))[0]
target_pattern = '^%s$' % get_target_name_from_plugin_path(args.plugin_path)
include_path = args.plugin_path
missing = True
target_name = get_target_name_from_plugin_path(args.plugin_path)
else:
# reporting on coverage exclusive to the matched targets
# the report can contain multiple targets
cache_path_format = '%s'
target_pattern = args.targets
include_path = None
missing = False
target_name = None

# identify integration test targets to analyze
target_names = sorted(combined['targets'])
incidental_target_names = [target for target in target_names if re.search(args.targets, target)]
incidental_target_names = [target for target in target_names if re.search(target_pattern, target)]

if not incidental_target_names:
raise ApplicationError('no targets to analyze')
if target_name:
# if the plugin has no tests we still want to know what coverage is missing
incidental_target_names = [target_name]
else:
raise ApplicationError('no targets to analyze')

# exclude test support plugins from analysis
# also exclude six, which for an unknown reason reports bogus coverage lines (indicating coverage of comments)
Expand All @@ -169,43 +199,80 @@ def incidental_report(args):
# process coverage for each target and then generate a report
# save sources for generating a summary report at the end
summary = {}
report_paths = {}

for target_name in incidental_target_names:
only_target_path = os.path.join(data_path, 'only-%s.json' % target_name)
cached(only_target_path, args.use_cache,
lambda: ct.filter(combined_path, only_target_path, include_targets=[target_name], exclude_path=exclude_path))
cache_name = cache_path_format % target_name

only_target_path = os.path.join(data_path, 'only-%s.json' % cache_name)
cached(only_target_path, args.use_cache, args.verbose,
lambda: ct.filter(combined_path, only_target_path, include_targets=[target_name], include_path=include_path, exclude_path=exclude_path))

without_target_path = os.path.join(data_path, 'without-%s.json' % target_name)
cached(without_target_path, args.use_cache,
lambda: ct.filter(combined_path, without_target_path, exclude_targets=[target_name], exclude_path=exclude_path))
without_target_path = os.path.join(data_path, 'without-%s.json' % cache_name)
cached(without_target_path, args.use_cache, args.verbose,
lambda: ct.filter(combined_path, without_target_path, exclude_targets=[target_name], include_path=include_path, exclude_path=exclude_path))

if missing:
source_target_path = missing_target_path = os.path.join(data_path, 'missing-%s.json' % cache_name)
cached(missing_target_path, args.use_cache, args.verbose,
lambda: ct.missing(without_target_path, only_target_path, missing_target_path, only_gaps=True))
else:
source_target_path = exclusive_target_path = os.path.join(data_path, 'exclusive-%s.json' % cache_name)
cached(exclusive_target_path, args.use_cache, args.verbose,
lambda: ct.missing(only_target_path, without_target_path, exclusive_target_path, only_gaps=True))

exclusive_target_path = os.path.join(data_path, 'exclusive-%s.json' % target_name)
cached(exclusive_target_path, args.use_cache,
lambda: ct.missing(only_target_path, without_target_path, exclusive_target_path, only_gaps=True))
source_expanded_target_path = os.path.join(os.path.dirname(source_target_path), 'expanded-%s' % os.path.basename(source_target_path))
cached(source_expanded_target_path, args.use_cache, args.verbose,
lambda: ct.expand(source_target_path, source_expanded_target_path))

exclusive_expanded_target_path = os.path.join(data_path, 'exclusive-expanded-%s.json' % target_name)
cached(exclusive_expanded_target_path, args.use_cache,
lambda: ct.expand(exclusive_target_path, exclusive_expanded_target_path))
summary[target_name] = sources = collect_sources(source_expanded_target_path, git, coverage_data)

summary[target_name] = sources = collect_sources(exclusive_expanded_target_path, git, coverage_data)
txt_report_path = os.path.join(reports_path, '%s.txt' % cache_name)
cached(txt_report_path, args.use_cache, args.verbose,
lambda: generate_report(sources, txt_report_path, coverage_data, target_name, missing=missing))

txt_report_path = os.path.join(reports_path, '%s.txt' % target_name)
cached(txt_report_path, args.use_cache,
lambda: generate_report(sources, txt_report_path, coverage_data, target_name))
report_paths[target_name] = txt_report_path

# provide a summary report of results
for target_name in incidental_target_names:
sources = summary[target_name]
report_path = os.path.relpath(report_paths[target_name])

print('%s: %d arcs, %d lines, %d files' % (
print('%s: %d arcs, %d lines, %d files - %s' % (
target_name,
sum(len(s.covered_arcs) for s in sources),
sum(len(s.covered_lines) for s in sources),
len(sources),
report_path,
))

sys.stderr.write('NOTE: This report shows only coverage exclusive to the reported targets. '
'As targets are removed, exclusive coverage on the remaining targets will increase.\n')
if not missing:
sys.stderr.write('NOTE: This report shows only coverage exclusive to the reported targets. '
'As targets are removed, exclusive coverage on the remaining targets will increase.\n')


def get_target_name_from_plugin_path(path): # type: (str) -> str
"""Return the integration test target name for the given plugin path."""
parts = os.path.splitext(path)[0].split(os.path.sep)
plugin_name = parts[-1]

if path.startswith('lib/ansible/modules/'):
plugin_type = None
elif path.startswith('lib/ansible/plugins/'):
plugin_type = parts[3]
elif path.startswith('lib/ansible/module_utils/'):
plugin_type = parts[2]
elif path.startswith('plugins/'):
plugin_type = parts[1]
else:
raise ApplicationError('Cannot determine plugin type from plugin path: %s' % path)

if plugin_type is None:
target_name = plugin_name
else:
target_name = '%s_%s' % (plugin_type, plugin_name)

return target_name


class CoverageData:
Expand Down Expand Up @@ -259,7 +326,7 @@ def __init__(self):
def combine(self, input_paths, output_path):
subprocess.check_call(self.analyze_cmd + ['combine'] + input_paths + [output_path])

def filter(self, input_path, output_path, include_targets=None, exclude_targets=None, exclude_path=None):
def filter(self, input_path, output_path, include_targets=None, exclude_targets=None, include_path=None, exclude_path=None):
args = []

if include_targets:
Expand All @@ -270,6 +337,9 @@ def filter(self, input_path, output_path, include_targets=None, exclude_targets=
for target in exclude_targets:
args.extend(['--exclude-target', target])

if include_path:
args.extend(['--include-path', include_path])

if exclude_path:
args.extend(['--exclude-path', exclude_path])

Expand Down Expand Up @@ -320,9 +390,9 @@ def collect_sources(data_path, git, coverage_data):
return sources


def generate_report(sources, report_path, coverage_data, target_name):
def generate_report(sources, report_path, coverage_data, target_name, missing):
output = [
'Target: %s' % target_name,
'Target: %s (%s coverage)' % (target_name, 'missing' if missing else 'exclusive'),
'GitHub: %stest/integration/targets/%s' % (coverage_data.github_base_url, target_name),
]

Expand Down Expand Up @@ -374,17 +444,22 @@ def parse_arc(value):
return tuple(int(v) for v in value.split(':'))


def cached(path, use_cache, func):
def cached(path, use_cache, show_messages, func):
if os.path.exists(path) and use_cache:
sys.stderr.write('%s: cached\n' % path)
sys.stderr.flush()
if show_messages:
sys.stderr.write('%s: cached\n' % path)
sys.stderr.flush()
return

sys.stderr.write('%s: generating ... ' % path)
sys.stderr.flush()
if show_messages:
sys.stderr.write('%s: generating ... ' % path)
sys.stderr.flush()

func()
sys.stderr.write('done\n')
sys.stderr.flush()

if show_messages:
sys.stderr.write('done\n')
sys.stderr.flush()


def check_failed(args, message):
Expand Down