Skip to content

Commit

Permalink
Allow to enable version_added checks for collections.
Browse files Browse the repository at this point in the history
Always validate version_added on module level if it is there.
Validate all version numbers via schema.
  • Loading branch information
felixfontein committed Mar 4, 2022
1 parent 7d234a4 commit 5c3542b
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 37 deletions.
24 changes: 14 additions & 10 deletions test/lib/ansible_test/_internal/cli/commands/sanity.py
Expand Up @@ -81,16 +81,20 @@ def do_sanity(
help='enable optional errors',
)

if data_context().content.is_ansible:
sanity.add_argument(
'--keep-git',
action='store_true',
help='transfer git related files to the remote host/container',
)
else:
sanity.set_defaults(
keep_git=False,
)
sanity.add_argument(
'--collection-version_added',
metavar='VERSION',
nargs='?',
default=None,
const='auto',
help='Enable version_added checks for collections. Optionally specify expected value to compare to',
)

sanity.add_argument(
'--keep-git',
action='store_true',
help='transfer git related files to the remote host/container',
)

sanity.add_argument(
'--lint',
Expand Down
Expand Up @@ -87,19 +87,31 @@ def test(self, args, targets, python): # type: (SanityConfig, SanityTargets, Py
'--arg-spec',
] + paths

needs_base_branch = False
if data_context().content.collection:
cmd.extend(['--collection', data_context().content.collection.directory])
if args.collection_version_added and args.collection_version_added != 'auto':
cmd.extend(['--collection-version_added', args.collection_version_added])
needs_base_branch = True

try:
collection_detail = get_collection_detail(args, python)

if collection_detail.version:
cmd.extend(['--collection-version', collection_detail.version])
if args.collection_version_added == 'auto':
cmd.extend(['--collection-version_added', collection_detail.version])
needs_base_branch = True
else:
display.warning('Skipping validate-modules collection version checks since no collection version was found.')
if args.collection_version_added == 'auto':
display.warning('Skipping validate-modules collection version_added checks since no collection version was found.')
except CollectionDetailError as ex:
display.warning('Skipping validate-modules collection version checks since collection detail loading failed: %s' % ex.reason)
else:
needs_base_branch = True

if needs_base_branch:
base_branch = args.base_branch or get_ci_provider().get_base_branch()

if base_branch:
Expand Down
1 change: 1 addition & 0 deletions test/lib/ansible_test/_internal/config.py
Expand Up @@ -250,6 +250,7 @@ def __init__(self, args): # type: (t.Any) -> None
self.list_tests = args.list_tests # type: bool
self.allow_disabled = args.allow_disabled # type: bool
self.enable_optional_errors = args.enable_optional_errors # type: bool
self.collection_version_added = args.collection_version_added # type: str
self.keep_git = args.keep_git # type: bool
self.prime_venvs = args.prime_venvs # type: bool

Expand Down
Expand Up @@ -303,7 +303,7 @@ class ModuleValidator(Validator):
ACCEPTLIST_FUTURE_IMPORTS = frozenset(('absolute_import', 'division', 'print_function'))

def __init__(self, path, analyze_arg_spec=False, collection=None, collection_version=None,
base_branch=None, git_cache=None, reporter=None, routing=None):
collection_version_added=None, base_branch=None, git_cache=None, reporter=None, routing=None):
super(ModuleValidator, self).__init__(reporter=reporter or Reporter())

self.path = path
Expand All @@ -328,6 +328,8 @@ def __init__(self, path, analyze_arg_spec=False, collection=None, collection_ver
self.collection_version_str = collection_version
self.collection_version = SemanticVersion(collection_version)

self.collection_version_added = collection_version_added

self.base_branch = base_branch
self.git_cache = git_cache or GitCache()

Expand Down Expand Up @@ -1022,7 +1024,7 @@ def _validate_docs(self):
'invalid-documentation',
)

if not self.collection:
if not self.collection or self.collection_version_added:
existing_doc = self._check_for_new_args(doc)
self._check_version_added(doc, existing_doc)

Expand Down Expand Up @@ -1151,17 +1153,17 @@ def _check_version_added(self, doc, existing_doc):
try:
collection_name = doc.get('version_added_collection')
version_added = self._create_strict_version(
str(version_added_raw or '0.0'),
str(version_added_raw or '0.0.0'),
collection_name=collection_name)
except ValueError as e:
version_added = version_added_raw or '0.0'
if self._is_new_module() or version_added != 'historical':
except ValueError:
version_added = version_added_raw or '0.0.0'
if self._is_new_module() or not (version_added == 'historical' and collection_name == 'ansible.builtin'):
# already reported during schema validation, except:
if version_added == 'historical':
if version_added == 'historical' and collection_name == 'ansible.builtin':
self.reporter.error(
path=self.object_path,
code='module-invalid-version-added',
msg='version_added is not a valid version number: %r. Error: %s' % (version_added, e)
msg='version_added is not a valid version number: %r' % 'historical'
)
return

Expand All @@ -1175,11 +1177,14 @@ def _check_version_added(self, doc, existing_doc):
if not self._is_new_module():
return

should_be = '.'.join(ansible_version.split('.')[:2])
strict_ansible_version = self._create_strict_version(should_be, collection_name='ansible.builtin')
if self.collection:
should_be = self.collection_version_added
else:
should_be = '.'.join(ansible_version.split('.')[:2])
should_be_strict = self._create_strict_version(should_be, collection_name=self.collection_name)

if (version_added < strict_ansible_version or
strict_ansible_version < version_added):
if (version_added < should_be_strict or
should_be_strict < version_added):
self.reporter.error(
path=self.object_path,
code='module-incorrect-version-added',
Expand Down Expand Up @@ -2030,16 +2035,21 @@ def _check_for_new_args(self, doc):
try:
mod_collection_name = existing_doc.get('version_added_collection')
mod_version_added = self._create_strict_version(
str(existing_doc.get('version_added', '0.0')),
str(existing_doc.get('version_added', '0.0.0')),
collection_name=mod_collection_name)
except ValueError:
mod_collection_name = self.collection_name
mod_version_added = self._create_strict_version('0.0')
mod_version_added = self._create_strict_version(
'0.0.0', collection_name=self.collection_name)

options = doc.get('options', {}) or {}

should_be = '.'.join(ansible_version.split('.')[:2])
strict_ansible_version = self._create_strict_version(should_be, collection_name='ansible.builtin')
if self.collection:
should_be = self.collection_version_added
else:
should_be = '.'.join(ansible_version.split('.')[:2])
should_be_strict = self._create_strict_version(
should_be, collection_name=self.collection_name)

for option, details in options.items():
try:
Expand Down Expand Up @@ -2078,17 +2088,17 @@ def _check_for_new_args(self, doc):
try:
collection_name = details.get('version_added_collection')
version_added = self._create_strict_version(
str(details.get('version_added', '0.0')),
str(details.get('version_added', '0.0.0')),
collection_name=collection_name)
except ValueError as e:
# already reported during schema validation
continue

if collection_name != self.collection_name:
continue
if (strict_ansible_version != mod_version_added and
(version_added < strict_ansible_version or
strict_ansible_version < version_added)):
if (should_be_strict != mod_version_added and
(version_added < should_be_strict or
should_be_strict < version_added)):
self.reporter.error(
path=self.object_path,
code='option-incorrect-version-added',
Expand Down Expand Up @@ -2311,13 +2321,16 @@ def run():
parser.add_argument('--collection-version',
help='The collection\'s version number used to check '
'deprecations')
parser.add_argument('--collection-version_added',
help='Enable version_added checks for collections; uses '
'given version to compare to')

args = parser.parse_args()

args.modules = [m.rstrip('/') for m in args.modules]

reporter = Reporter()
git_cache = GitCache(args.base_branch)
git_cache = GitCache(args.base_branch, collection=args.collection)

check_dirs = set()

Expand All @@ -2342,6 +2355,7 @@ def run():
if ModuleValidator.is_on_rejectlist(path):
continue
with ModuleValidator(path, collection=args.collection, collection_version=args.collection_version,
collection_version_added=args.collection_version_added,
analyze_arg_spec=args.arg_spec, base_branch=args.base_branch,
git_cache=git_cache, reporter=reporter, routing=routing) as mv1:
mv1.validate()
Expand All @@ -2366,6 +2380,7 @@ def run():
if ModuleValidator.is_on_rejectlist(path):
continue
with ModuleValidator(path, collection=args.collection, collection_version=args.collection_version,
collection_version_added=args.collection_version_added,
analyze_arg_spec=args.arg_spec, base_branch=args.base_branch,
git_cache=git_cache, reporter=reporter, routing=routing) as mv2:
mv2.validate()
Expand All @@ -2382,16 +2397,20 @@ def run():


class GitCache:
def __init__(self, base_branch):
def __init__(self, base_branch, collection=None):
self.base_branch = base_branch

self.base_path = 'lib/ansible/modules/'
if collection:
self.base_path = 'plugins/modules/'

if self.base_branch:
self.base_tree = self._git(['ls-tree', '-r', '--name-only', self.base_branch, 'lib/ansible/modules/'])
self.base_tree = self._git(['ls-tree', '-r', '--name-only', self.base_branch, self.base_path])
else:
self.base_tree = []

try:
self.head_tree = self._git(['ls-tree', '-r', '--name-only', 'HEAD', 'lib/ansible/modules/'])
self.head_tree = self._git(['ls-tree', '-r', '--name-only', 'HEAD', self.base_path])
except GitError as ex:
if ex.status == 128:
# fallback when there is no .git directory
Expand All @@ -2418,11 +2437,10 @@ def __init__(self, base_branch):
if os.path.islink(path):
self.head_aliased_modules.add(os.path.basename(os.path.realpath(path)))

@staticmethod
def _get_module_files():
def _get_module_files(self):
module_files = []

for (dir_path, dir_names, file_names) in os.walk('lib/ansible/modules/'):
for (dir_path, dir_names, file_names) in os.walk(self.base_path):
for file_name in file_names:
module_files.append(os.path.join(dir_path, file_name))

Expand Down

0 comments on commit 5c3542b

Please sign in to comment.