Refactor rspec-tools and other cleanups

The main changes are:
 * Split RuleCreator: move some of its content to RspecRepo and to
   RuleEditor in new modules.
 * Refactor tests accordingly.

Other less important changes:
 * Sort and remove unnecessary imports
 * Remove unimplemented functions and unnecessary classes
 * Make some functions private
 * Move pushd from utils to tests where it is only used
 * Reduce code duplication here and there
 * Remove unnecessary Mock in some tests
 * Improve coverage for add_language_to_rule
This commit is contained in:
Marco Antognini 2022-02-24 17:09:07 +01:00 committed by Marco Antognini
parent dbb8027666
commit 26e3ebc7ec
14 changed files with 591 additions and 501 deletions

View File

@ -1,30 +1,34 @@
import os
from typing import Optional
from pathlib import Path
from typing import Optional
import click
from rspec_tools.checklinks import check_html_links
from rspec_tools.errors import RuleNotFoundError, RuleValidationError
from rspec_tools.rules import RulesRepository, LanguageSpecificRule
from rspec_tools.validation.metadata import validate_rule_metadata
from rspec_tools.validation.description import validate_section_names, validate_section_levels, validate_parameters, validate_source_language
from rspec_tools.coverage import update_coverage_for_all_repos, update_coverage_for_repo, update_coverage_for_repo_version
import rspec_tools.create_rule
import rspec_tools.create_rule
import rspec_tools.modify_rule
from rspec_tools.checklinks import check_html_links
from rspec_tools.coverage import (update_coverage_for_all_repos,
update_coverage_for_repo,
update_coverage_for_repo_version)
from rspec_tools.errors import RuleValidationError
from rspec_tools.notify_failure_on_slack import notify_slack
from rspec_tools.rules import LanguageSpecificRule, RulesRepository
from rspec_tools.validation.description import (validate_parameters,
validate_section_levels,
validate_section_names,
validate_source_language)
from rspec_tools.validation.metadata import validate_rule_metadata
def _fatal_error(message: str):
click.echo(message, err=True)
raise click.Abort(message)
@click.group()
@click.option('--debug/--no-debug', default=False)
def cli(debug):
'Tools automating RSPEC workflows.'
@cli.command()
@click.option('--rule', help='Validate only the rule matching the provided ID.')
def validate(rule):
'''Validate rules.'''
# TODO
if rule == '42':
raise RuleNotFoundError(rule)
@cli.command()
@click.option('--d', required=True)
@ -32,6 +36,7 @@ def check_links(d):
'''Check links in html.'''
check_html_links(d)
@cli.command()
@click.option('--languages', required=True)
@click.option('--user', required=False)
@ -40,6 +45,7 @@ def create_rule(languages: str, user: Optional[str]):
token = os.environ.get('GITHUB_TOKEN')
rspec_tools.create_rule.create_new_rule(languages, token, user)
@cli.command()
@click.option('--language', required=True)
@click.option('--rule', required=True)
@ -49,6 +55,7 @@ def add_lang_to_rule(language: str, rule: str, user: Optional[str]):
token = os.environ.get('GITHUB_TOKEN')
rspec_tools.create_rule.add_language_to_rule(language, rule, token, user)
@cli.command()
@click.option('--language', required=True)
@click.option('--rule', required=True)
@ -57,7 +64,8 @@ def add_lang_to_rule(language: str, rule: str, user: Optional[str]):
def update_quickfix_status(language: str, rule: str, status: str, user: Optional[str]):
'''Update the status of quick fix for the given rule/language'''
token = os.environ.get('GITHUB_TOKEN')
rspec_tools.create_rule.update_rule_quickfix_status(language, rule, status, token, user)
rspec_tools.modify_rule.update_rule_quickfix_status(language, rule, status, token, user)
@cli.command()
@click.argument('rules', nargs=-1, required=True)
@ -75,7 +83,7 @@ def validate_rules_metadata(rules):
error_counter += 1
if error_counter > 0:
fatal_error(f"Validation failed due to {error_counter} errors out of {len(rules)} analyzed rules")
_fatal_error(f"Validation failed due to {error_counter} errors out of {len(rules)} analyzed rules")
VALIDATORS = [validate_section_names,
@ -83,7 +91,7 @@ VALIDATORS = [validate_section_names,
validate_parameters,
validate_source_language,
]
def validate_rule_specialization(lang_spec_rule: LanguageSpecificRule):
def _validate_rule_specialization(lang_spec_rule: LanguageSpecificRule):
error_counter = 0
for validator in VALIDATORS:
try:
@ -93,6 +101,7 @@ def validate_rule_specialization(lang_spec_rule: LanguageSpecificRule):
error_counter += 1
return error_counter
@cli.command()
@click.option('--d', required=True)
@click.argument('rules', nargs=-1)
@ -105,9 +114,10 @@ def check_description(d, rules):
if rules and rule.id not in rules:
continue
for lang_spec_rule in rule.specializations:
error_counter += validate_rule_specialization(lang_spec_rule)
error_counter += _validate_rule_specialization(lang_spec_rule)
if error_counter > 0:
fatal_error(f"Validation failed due to {error_counter} errors")
_fatal_error(f"Validation failed due to {error_counter} errors")
@cli.command()
@click.option('--repository', required=False)
@ -121,14 +131,12 @@ def update_coverage(repository: Optional[str], version: Optional[str]):
else:
update_coverage_for_repo_version(repository, version)
@cli.command()
@click.option('--message', required=True)
@click.option('--channel', required=True)
def notify_failure_on_slack(message: str, channel: str):
notify_slack(message, channel)
__all__=['cli']
def fatal_error(message: str):
click.echo(message, err=True)
raise click.Abort(message)
__all__=['cli']

View File

@ -1,132 +1,57 @@
import json
import os
import tempfile
from contextlib import contextmanager
from pathlib import Path
from typing import Callable, Final, Iterable, Optional
import click
from git import Repo
from github import Github
from github.PullRequest import PullRequest
from rspec_tools.errors import InvalidArgumentError
from rspec_tools.repo import RspecRepo, tmp_rspec_repo
from rspec_tools.utils import (LANG_TO_SOURCE, copy_directory_content,
get_label_for_language,
get_labels_for_languages, is_empty_metadata,
parse_and_validate_language_list, resolve_rule,
swap_metadata_files, validate_language)
swap_metadata_files)
def build_github_repository_url(token: str, user: Optional[str]):
'''Builds the rspec repository url'''
repo = os.environ.get('GITHUB_REPOSITORY', 'SonarSource/rspec')
if user:
return f'https://{user}:{token}@github.com/{repo}.git'
else:
return f'https://{token}@github.com/{repo}.git'
def extract_repository_name(url):
url_end = url.split('/')[-2:]
return '/'.join(url_end).removesuffix('.git')
def auto_github(token: str) -> Callable[[Optional[str]], Github]:
def ret(user: Optional[str]):
if user:
return Github(user, token)
else:
return Github(token)
return ret
def _get_url_and_config(token: str, user: Optional[str]):
url = build_github_repository_url(token, user)
config = {}
if user:
config['user.name'] = user
config['user.email'] = f'{user}@users.noreply.github.com'
return url, config
def _get_valid_label_for_language(language: str):
validate_language(language)
return get_label_for_language(language)
@contextmanager
def _rule_creator(token: str, user: Optional[str]):
url, config = _get_url_and_config(token, user)
with tempfile.TemporaryDirectory() as tmpdirname:
rule_creator = RuleCreator(url, tmpdirname, config)
yield rule_creator
with tmp_rspec_repo(token, user) as repo:
yield RuleCreator(repo)
def create_new_rule(languages: str, token: str, user: Optional[str]):
lang_list = parse_and_validate_language_list(languages)
label_list = get_labels_for_languages(lang_list)
with _rule_creator(token, user) as rule_creator:
rule_number = rule_creator.reserve_rule_number()
rule_creator.create_new_rule_pull_request(auto_github(token), rule_number, lang_list, label_list, user=user)
rule_number = rule_creator.rspec_repo.reserve_rule_number()
rule_creator.create_new_rule_pull_request(token, rule_number, lang_list, label_list, user)
def add_language_to_rule(language: str, rule: str, token: str, user: Optional[str]):
label = _get_valid_label_for_language(language)
label = get_label_for_language(language)
rule_number = resolve_rule(rule)
with _rule_creator(token, user) as rule_creator:
rule_creator.add_language_pull_request(auto_github(token), rule_number, language, label, user=user)
rule_creator.add_language_pull_request(token, rule_number, language, label, user)
def update_rule_quickfix_status(language: str, rule: str, status: str, token: str, user: Optional[str]):
label = _get_valid_label_for_language(language)
rule_number = resolve_rule(rule)
with _rule_creator(token, user) as rule_creator:
rule_creator.update_quickfix_status_pull_request(auto_github(token), rule_number, language, status, label, user)
class RuleCreator:
'''Create a new Rule in a repository following the official Github 'rspec' repository structure.'''
MASTER_BRANCH: Final[str] = 'master'
ID_COUNTER_BRANCH: Final[str] = 'rspec-id-counter'
ID_COUNTER_FILENAME: Final[str] = 'next_rspec_id.txt'
TEMPLATE_PATH: Final[Path] = Path(__file__).parent.parent.joinpath('rspec_template')
repository: Final[Repo]
origin_url: Final[str]
def __init__(self, repository_url: str, clone_directory: str, configuration: dict[str, str]):
self.repository = Repo.clone_from(repository_url, clone_directory)
self.origin_url = repository_url
# create local branches tracking remote ones
for branch in [self.MASTER_BRANCH, self.ID_COUNTER_BRANCH]:
self.repository.remote().fetch(branch)
self.repository.git.checkout('-B', branch, f'origin/{branch}')
# update repository config
with self.repository.config_writer() as config_writer:
for key, value in configuration.items():
split_key = key.split('.')
config_writer.set_value(*split_key, value)
def reserve_rule_number(self) -> int:
'''Reserve an id on the id counter branch of the repository.'''
with self._current_git_branch(self.ID_COUNTER_BRANCH):
counter_file_path = Path(self.repository.working_dir).joinpath(self.ID_COUNTER_FILENAME)
counter = int(counter_file_path.read_text())
counter_file_path.write_text(str(counter + 1))
self.repository.index.add([str(counter_file_path)])
self.repository.index.commit('Increment RSPEC ID counter')
origin = self.repository.remote(name='origin')
origin.push()
click.echo(f'Reserved Rule ID S{counter}')
return counter
def __init__(self, rspec_repo: RspecRepo):
self.rspec_repo = rspec_repo
self.repo_dir = Path(self.rspec_repo.repository.working_dir)
def add_language_branch(self, rule_number: int, language: str) -> str:
'''Create and move files to add a new language to an existing rule.'''
branch_name = f'rule/S{rule_number}-add-{language}'
with self._current_git_branch(self.MASTER_BRANCH, branch_name):
repo_dir = Path(self.repository.working_dir)
rule_dir = repo_dir.joinpath('rules', f'S{rule_number}')
with self.rspec_repo.checkout_branch(self.rspec_repo.MASTER_BRANCH, branch_name):
rule_dir = self.repo_dir / 'rules' / f'S{rule_number}'
if not rule_dir.is_dir():
raise InvalidArgumentError(f"Rule \"S{rule_number}\" does not exist.")
lang_dir = rule_dir.joinpath(language)
lang_dir = rule_dir / language
if lang_dir.is_dir():
lang_url = f"https://github.com/SonarSource/rspec/tree/master/rules/S{rule_number}/{language}"
raise InvalidArgumentError(f"Rule \"S{rule_number}\" is already defined for language {language}. Modify the definition here: {lang_url}")
@ -135,66 +60,28 @@ class RuleCreator:
swap_metadata_files(rule_dir, lang_dirs[0])
lang_dir.mkdir()
lang_specific_template = self.TEMPLATE_PATH.joinpath('multi_language', 'language_specific')
lang_specific_template = self.TEMPLATE_PATH / 'multi_language' / 'language_specific'
copy_directory_content(lang_specific_template, lang_dir)
self._fill_in_the_blanks_in_the_template(lang_dir, rule_number)
self._fill_language_name_in_the_template(lang_dir, language)
self.repository.git.add('--all')
self.repository.index.commit(f'Add {language} to rule S{rule_number}')
self.repository.git.push('origin', branch_name)
self.rspec_repo.commit_all_and_push(f'Add {language} to rule S{rule_number}')
return branch_name
def create_new_rule_branch(self, rule_number: int, languages: Iterable[str]) -> str:
'''Create all the files required for a new rule.'''
branch_name = f'rule/add-RSPEC-S{rule_number}'
with self._current_git_branch(self.MASTER_BRANCH, branch_name):
repo_dir = Path(self.repository.working_dir)
rule_dir = repo_dir.joinpath('rules', f'S{rule_number}')
with self.rspec_repo.checkout_branch(self.rspec_repo.MASTER_BRANCH, branch_name):
rule_dir = self.repo_dir / 'rules' / f'S{rule_number}'
rule_dir.mkdir()
lang_count = sum(1 for _ in languages)
if lang_count > 1:
self._fill_multi_lang_template_files(rule_dir, rule_number, languages)
else:
self._fill_single_lang_template_files(rule_dir, rule_number, next(iter(languages)))
self.rspec_repo.commit_all_and_push(f'Create rule S{rule_number}')
self.repository.git.add('--all')
self.repository.index.commit(f'Create rule S{rule_number}')
self.repository.git.push('origin', branch_name)
return branch_name
def update_quickfix_status_branch(self, title: str, rule_number: int, language: str, status: str) -> str:
'''Update the given rule/language quick fix metadata field.'''
branch_name = f'rule/S{rule_number}-{language}-quickfix'
with self._current_git_branch(self.MASTER_BRANCH, branch_name):
self._update_quickfix_status(rule_number, language, status)
self.repository.git.add('--all')
self.repository.index.commit(title)
self.repository.git.push('origin', branch_name)
return branch_name
def _get_generic_quickfix_status(self, rule_number: int):
DEFAULT = 'unknown'
generic_metadata_path = Path(self.repository.working_dir, 'rules', f'S{rule_number}', 'metadata.json')
if not generic_metadata_path.is_file():
return DEFAULT
generic_metadata = json.loads(generic_metadata_path.read_text())
return generic_metadata.get('quickfix', DEFAULT)
def _update_quickfix_status(self, rule_number: int, language: str, status: str):
metadata_path = Path(self.repository.working_dir, 'rules', f'S{rule_number}', language, 'metadata.json')
if not metadata_path.is_file():
raise InvalidArgumentError(f'{metadata_path} does not exist or is not a file')
metadata = json.loads(metadata_path.read_text())
generic_status = self._get_generic_quickfix_status(rule_number)
if status == metadata.get('quickfix', generic_status):
raise InvalidArgumentError(f'{metadata_path} has already the same status {status}')
metadata['quickfix'] = status
# When generating the JSON, ensure forward slashes are escaped. See RULEAPI-750.
json_string = json.dumps(metadata, indent=2).replace("/", "\\/")
metadata_path.write_text(json_string)
def _fill_in_the_blanks_in_the_template(self, rule_dir: Path, rule_number: int):
for rule_item in rule_dir.glob('**/*'):
if rule_item.is_file():
@ -211,12 +98,12 @@ class RuleCreator:
rule_item.write_text(final_content)
def _fill_multi_lang_template_files(self, rule_dir: Path, rule_number: int, languages: Iterable[str]):
common_template = self.TEMPLATE_PATH.joinpath('multi_language', 'common')
lang_specific_template = self.TEMPLATE_PATH.joinpath('multi_language', 'language_specific')
common_template = self.TEMPLATE_PATH / 'multi_language' / 'common'
lang_specific_template = self.TEMPLATE_PATH / 'multi_language' / 'language_specific'
copy_directory_content(common_template, rule_dir)
for lang in languages:
lang_dir = rule_dir.joinpath(lang)
lang_dir = rule_dir / lang
lang_dir.mkdir()
copy_directory_content(lang_specific_template, lang_dir)
self._fill_language_name_in_the_template(lang_dir, lang)
@ -224,41 +111,22 @@ class RuleCreator:
self._fill_in_the_blanks_in_the_template(rule_dir, rule_number)
def _fill_single_lang_template_files(self, rule_dir: Path, rule_number: int, language: str):
common_template = self.TEMPLATE_PATH.joinpath('single_language', 'common')
lang_specific_template = self.TEMPLATE_PATH.joinpath('single_language', 'language_specific')
common_template = self.TEMPLATE_PATH / 'single_language' / 'common'
lang_specific_template = self.TEMPLATE_PATH / 'single_language' / 'language_specific'
copy_directory_content(common_template, rule_dir)
lang_dir = rule_dir.joinpath(language)
lang_dir = rule_dir /language
lang_dir.mkdir()
copy_directory_content(lang_specific_template, lang_dir)
self._fill_in_the_blanks_in_the_template(rule_dir, rule_number)
self._fill_language_name_in_the_template(lang_dir, language)
def _create_pull_request(self, github_api: Callable[[Optional[str]], Github], branch_name: str, title: str, body: str, labels: Iterable[str], user: Optional[str]):
repository_url = extract_repository_name(self.origin_url)
github = github_api(user)
github_repo = github.get_repo(repository_url)
pull_request = github_repo.create_pull(
title=title,
body=body,
head=branch_name, base=self.MASTER_BRANCH,
draft=True, maintainer_can_modify=True
)
click.echo(f'Created rule Pull Request {pull_request.html_url}')
# Note: It is not possible to get the authenticated user using get_user() from a github action.
login = user if user else github.get_user().login
pull_request.add_to_assignees(login)
pull_request.add_to_labels(*labels)
click.echo(f'Pull request assigned to {login}')
return pull_request
def add_language_pull_request(self, github_api: Callable[[Optional[str]], Github], rule_number: int, language: str, label: str, user: Optional[str]):
def add_language_pull_request(self, token: str, rule_number: int, language: str, label: str, user: Optional[str]):
branch_name = self.add_language_branch(rule_number, language)
click.echo(f'Created rule branch {branch_name}')
return self._create_pull_request(
github_api,
return self.rspec_repo.create_pull_request(
token,
branch_name,
f'Create rule S{rule_number}',
f'You can preview this rule [here](https://sonarsource.github.io/rspec/#/rspec/S{rule_number}/{language}) (updated a few minutes after each push).',
@ -266,12 +134,12 @@ class RuleCreator:
user
)
def create_new_rule_pull_request(self, github_api: Callable[[Optional[str]], Github], rule_number: int, languages: Iterable[str], labels: Iterable[str], *, user: Optional[str]) -> PullRequest:
def create_new_rule_pull_request(self, token: str, rule_number: int, languages: Iterable[str], labels: Iterable[str], user: Optional[str]) -> PullRequest:
branch_name = self.create_new_rule_branch(rule_number, languages)
click.echo(f'Created rule branch {branch_name}')
first_lang = next(iter(languages))
return self._create_pull_request(
github_api,
return self.rspec_repo.create_pull_request(
token,
branch_name,
f'Create rule S{rule_number}',
f'You can preview this rule [here](https://sonarsource.github.io/rspec/#/rspec/S{rule_number}/{first_lang}) (updated a few minutes after each push).',
@ -279,31 +147,3 @@ class RuleCreator:
user
)
def update_quickfix_status_pull_request(self, github_api: Callable[[Optional[str]], Github], rule_number: int, language: str, status: str, label: str, user: Optional[str]):
title = f'Modify rule S{rule_number}: mark quick fix as "{status}"'
branch_name = self.update_quickfix_status_branch(title, rule_number, language, status)
click.echo(f'Created rule branch {branch_name}')
return self._create_pull_request(
github_api,
branch_name,
title,
f'''See the original rule [here](https://sonarsource.github.io/rspec/#/rspec/S{rule_number}/{language}).
The rule won't be updated until this PR is merged, see [RULEAPI-655](https://jira.sonarsource.com/browse/RULEAPI-655)''',
[label],
user
)
@contextmanager
def _current_git_branch(self, base_branch: str, new_branch: Optional[str] = None):
'''Checkout a given branch before yielding, then revert to the previous branch.'''
past_branch = self.repository.active_branch
try:
self.repository.git.checkout(base_branch)
origin = self.repository.remote(name='origin')
origin.pull()
if new_branch is not None:
self.repository.git.checkout('-B', new_branch)
yield
finally:
self.repository.git.checkout(past_branch)

View File

@ -1,20 +1,15 @@
from click import ClickException
class RuleNotFoundError(ClickException):
def __init__(self, id):
super().__init__(f'No rule has ID {id}')
class InvalidArgumentError(ClickException):
'''Exception raised when an invalid argument is given to a CLI command.'''
def __init__(self, message):
super().__init__(message)
class GitError(ClickException):
'''Exception raised when some error happened with git commands.'''
def __init__(self, message):
super().__init__(message)
class RuleValidationError(ClickException):
'''Exception raised when a rule did not pass validation.'''
def __init__(self, message):
super().__init__(message)

View File

@ -0,0 +1,78 @@
from contextlib import contextmanager
import json
from pathlib import Path
from typing import Optional
import click
from rspec_tools.errors import InvalidArgumentError
from rspec_tools.repo import RspecRepo, tmp_rspec_repo
from rspec_tools.utils import get_label_for_language, resolve_rule
@contextmanager
def _rule_editor(token: str, user: Optional[str]):
with tmp_rspec_repo(token, user) as repo:
yield RuleEditor(repo)
def update_rule_quickfix_status(language: str, rule: str, status: str, token: str, user: Optional[str]):
label = get_label_for_language(language)
rule_number = resolve_rule(rule)
with _rule_editor(token, user) as editor:
editor.update_quickfix_status_pull_request(token, rule_number, language, status, label, user)
class RuleEditor:
'''Modify an existing Rule in a repository following the official Github 'rspec' repository structure.'''
def __init__(self, rspec_repo: RspecRepo):
self.rspec_repo = rspec_repo
self.repo_dir = Path(self.rspec_repo.repository.working_dir)
def update_quickfix_status_branch(self, title: str, rule_number: int, language: str, status: str) -> str:
'''Update the given rule/language quick fix metadata field.'''
branch_name = f'rule/S{rule_number}-{language}-quickfix'
with self.rspec_repo.checkout_branch(self.rspec_repo.MASTER_BRANCH, branch_name):
self._update_quickfix_status(rule_number, language, status)
self.rspec_repo.commit_all_and_push(title)
return branch_name
def update_quickfix_status_pull_request(self, token: str, rule_number: int, language: str, status: str, label: str, user: Optional[str]):
title = f'Modify rule S{rule_number}: mark quick fix as "{status}"'
branch_name = self.update_quickfix_status_branch(title, rule_number, language, status)
click.echo(f'Created rule branch {branch_name}')
return self.rspec_repo.create_pull_request(
token,
branch_name,
title,
f'''See the original rule [here](https://sonarsource.github.io/rspec/#/rspec/S{rule_number}/{language}).
The rule won't be updated until this PR is merged, see [RULEAPI-655](https://jira.sonarsource.com/browse/RULEAPI-655)''',
[label],
user
)
def _get_generic_quickfix_status(self, rule_number: int):
DEFAULT = 'unknown'
generic_metadata_path = self.repo_dir / 'rules' / f'S{rule_number}' / 'metadata.json'
if not generic_metadata_path.is_file():
return DEFAULT
generic_metadata = json.loads(generic_metadata_path.read_text())
return generic_metadata.get('quickfix', DEFAULT)
def _update_quickfix_status(self, rule_number: int, language: str, status: str):
metadata_path = self.repo_dir / 'rules' / f'S{rule_number}' / language / 'metadata.json'
if not metadata_path.is_file():
raise InvalidArgumentError(f'{metadata_path} does not exist or is not a file')
metadata = json.loads(metadata_path.read_text())
generic_status = self._get_generic_quickfix_status(rule_number)
if status == metadata.get('quickfix', generic_status):
raise InvalidArgumentError(f'{metadata_path} has already the same status {status}')
metadata['quickfix'] = status
# When generating the JSON, ensure forward slashes are escaped. See RULEAPI-750.
json_string = json.dumps(metadata, indent=2).replace("/", "\\/")
metadata_path.write_text(json_string)

View File

@ -0,0 +1,130 @@
import os
import tempfile
from contextlib import contextmanager
from pathlib import Path
from typing import Callable, Final, Iterable, Optional
import click
from git import Repo
from github import Github
def _auto_github(token: str) -> Callable[[Optional[str]], Github]:
def ret(user: Optional[str]):
if user:
return Github(user, token)
else:
return Github(token)
return ret
class RspecRepo:
'''Provide operations on a git repository for rule specifications.'''
MASTER_BRANCH: Final[str] = 'master'
ID_COUNTER_BRANCH: Final[str] = 'rspec-id-counter'
ID_COUNTER_FILENAME: Final[str] = 'next_rspec_id.txt'
repository: Final[Repo]
origin_url: Final[str]
def __init__(self, origin_url: str, clone_directory: str, configuration: dict[str, str]):
self.repository = Repo.clone_from(origin_url, clone_directory)
self.origin_url = origin_url
# Create local branches tracking remote ones
for branch in [self.MASTER_BRANCH, self.ID_COUNTER_BRANCH]:
self.repository.remote().fetch(branch)
self.repository.git.checkout('-B', branch, f'origin/{branch}')
# Update repository config
with self.repository.config_writer() as config_writer:
for key, value in configuration.items():
split_key = key.split('.')
config_writer.set_value(*split_key, value)
def get_repository_name(self):
url_end = self.origin_url.split('/')[-2:]
return '/'.join(url_end).removesuffix('.git')
@contextmanager
def checkout_branch(self, base_branch: str, new_branch: Optional[str] = None):
'''Checkout a given branch before yielding, then revert to the previous branch.'''
past_branch = self.repository.active_branch
try:
self.repository.git.checkout(base_branch)
origin = self.repository.remote(name='origin')
origin.pull()
if new_branch is not None:
self.repository.git.checkout('-B', new_branch)
yield
finally:
self.repository.git.checkout(past_branch)
def commit_all_and_push(self, message: str):
self.repository.git.add('--all')
self.repository.index.commit(message)
self.repository.git.push('origin', self.repository.active_branch)
def create_pull_request(self, token: str, branch_name: str, title: str, body: str, labels: Iterable[str], user: Optional[str]):
'''Create a pull request from the given branch.'''
repository_url = self.get_repository_name()
github_api = _auto_github(token)
github = github_api(user)
github_repo = github.get_repo(repository_url)
pull_request = github_repo.create_pull(
title=title,
body=body,
head=branch_name, base=self.MASTER_BRANCH,
draft=True, maintainer_can_modify=True
)
click.echo(f'Created rule Pull Request {pull_request.html_url}')
# Note: It is not possible to get the authenticated user using get_user() from a github action.
login = user if user else github.get_user().login
pull_request.add_to_assignees(login)
pull_request.add_to_labels(*labels)
click.echo(f'Pull request assigned to {login}')
return pull_request
def reserve_rule_number(self) -> int:
'''Reserve an id on the id counter branch of the repository.'''
with self.checkout_branch(self.ID_COUNTER_BRANCH):
counter_file_path = Path(self.repository.working_dir, self.ID_COUNTER_FILENAME)
counter = int(counter_file_path.read_text())
counter_file_path.write_text(str(counter + 1))
self.repository.index.add([str(counter_file_path)])
self.repository.index.commit('Increment RSPEC ID counter')
origin = self.repository.remote(name='origin')
origin.push()
click.echo(f'Reserved Rule ID S{counter}')
return counter
def _build_github_repository_url(token: str, user: Optional[str]):
'''Builds the rspec repository url'''
repo = os.environ.get('GITHUB_REPOSITORY', 'SonarSource/rspec')
if user:
return f'https://{user}:{token}@github.com/{repo}.git'
else:
return f'https://{token}@github.com/{repo}.git'
def _get_url_and_config(token: str, user: Optional[str]):
url = _build_github_repository_url(token, user)
config = {}
if user:
config['user.name'] = user
config['user.email'] = f'{user}@users.noreply.github.com'
return url, config
@contextmanager
def tmp_rspec_repo(token: str, user: Optional[str]):
'''Yield a temporary repository'''
url, config = _get_url_and_config(token, user)
with tempfile.TemporaryDirectory() as tmpdirname:
yield RspecRepo(url, tmpdirname, config)

View File

@ -126,7 +126,7 @@ def parse_and_validate_language_list(languages):
_validate_languages(lang_list)
return lang_list
def validate_language(language):
def _validate_language(language):
_validate_languages([language])
def get_labels_for_languages(lang_list):
@ -134,6 +134,7 @@ def get_labels_for_languages(lang_list):
return list(set(labels))
def get_label_for_language(language: str) -> str:
_validate_language(language)
return LANG_TO_LABEL[language]
def resolve_rule(rule_id: str) -> int:
@ -146,12 +147,3 @@ def resolve_rule(rule_id: str) -> int:
def load_json(file):
with open(file) as json_file:
return json.load(json_file)
@contextlib.contextmanager
def pushd(new_dir):
previous_dir = os.getcwd()
os.chdir(new_dir)
try:
yield
finally:
os.chdir(previous_dir)

View File

@ -1,6 +1,11 @@
import shutil
from contextlib import contextmanager
from pathlib import Path
from unittest.mock import Mock, patch
import pytest
from git import Head, Repo
from rspec_tools.repo import RspecRepo
@pytest.fixture
@ -8,7 +13,69 @@ def mockrules():
'''Provides a path to test rules resources.'''
return Path(__file__).parent.joinpath('resources', 'rules')
@pytest.fixture
def mockinvalidrules():
'''Provides a path to test rules resources.'''
return Path(__file__).parent.joinpath('resources', 'invalid-rules')
@pytest.fixture
def git_config():
'''Create a mock git configuration.'''
return {
'user.name': 'testuser',
'user.email': 'testuser@mock.mock'
}
@pytest.fixture
def mock_git_rspec_repo(tmpdir, mockrules: Path):
repo_dir = tmpdir.mkdir("mock_rspec")
repo = Repo.init(str(repo_dir))
repo.init()
rules_dir = repo_dir.join('rules')
shutil.copytree(mockrules, rules_dir)
with repo.config_writer() as config_writer:
config_writer.set_value('user', 'name', 'originuser')
config_writer.set_value('user', 'email', 'originuser@mock.mock')
repo.git.add('--all')
repo.index.commit('init rules')
# Create the id counter branch. Note that it is an orphan branch.
repo.head.reference = Head(repo, f'refs/heads/{RspecRepo.ID_COUNTER_BRANCH}')
repo.git.reset('--hard')
counter_file = repo_dir.join(RspecRepo.ID_COUNTER_FILENAME)
counter_file.write('0')
repo.index.add([str(counter_file)])
commit = repo.index.commit('init counter', parent_commits=None)
# Checkout a specific commit so that the repo can be pushed to without
# making the index and work tree inconsistent.
repo.git.checkout(commit.hexsha)
return repo
@pytest.fixture
def rspec_repo(tmpdir, mock_git_rspec_repo: Repo, git_config: dict[str, str]):
cloned_repo = tmpdir.mkdir("cloned_repo")
return RspecRepo(mock_git_rspec_repo.working_dir, str(cloned_repo), git_config)
@contextmanager
def mock_github():
token = 'TOKEN'
user = 'testuser'
mock_repo = Mock()
mock_github = Mock()
mock_github.get_repo = Mock(return_value=mock_repo)
mock_github_api = Mock(return_value=mock_github)
mock_auto_github = Mock(return_value=mock_github_api)
with patch('rspec_tools.repo._auto_github', mock_auto_github):
yield (token, user, mock_repo)
mock_auto_github.assert_called_once_with(token)
mock_github_api.assert_called_once_with(user)
mock_github.get_repo.assert_called_once()

View File

@ -2,19 +2,18 @@ import os
import re
from pathlib import Path
from typing import List
from unittest.mock import Mock, patch
from unittest.mock import patch
from click.testing import CliRunner
from rspec_tools.cli import cli
from rspec_tools.rules import RulesRepository
import rspec_tools
class TestCLIUpdateQuickfixStatus:
'''Unit test for quickfix status update through Command Line Interface.'''
@patch.dict(os.environ, {'GITHUB_TOKEN': 'TOKEN'})
@patch('rspec_tools.create_rule.update_rule_quickfix_status')
@patch('rspec_tools.modify_rule.update_rule_quickfix_status')
def test_basic_cli_usage(self, mock):
arguments = [
'update-quickfix-status',

View File

@ -1,10 +1,22 @@
from git import Repo, Head
from unittest.mock import Mock, patch
import pytest
import json
import os
from contextlib import contextmanager
from unittest.mock import patch
from rspec_tools.coverage import (update_coverage_for_all_repos,
update_coverage_for_repo,
update_coverage_for_repo_version)
from rspec_tools.utils import load_json
@contextmanager
def pushd(new_dir):
previous_dir = os.getcwd()
os.chdir(new_dir)
try:
yield
finally:
os.chdir(previous_dir)
from rspec_tools.coverage import update_coverage_for_repo_version, update_coverage_for_repo, update_coverage_for_all_repos
from rspec_tools.utils import load_json, pushd
def test_update_coverage_for_repo_version(tmpdir):
with pushd(tmpdir):
@ -36,6 +48,7 @@ def test_update_coverage_for_repo_version(tmpdir):
update_coverage_for_repo_version(REPO, 'master')
assert load_json(coverage)['JAVASCRIPT']['S100'] == REPO + ' ' + VER
def test_update_coverage_for_repo(tmpdir):
with pushd(tmpdir):
REPO = 'SonarJS'
@ -50,6 +63,7 @@ def test_update_coverage_for_repo(tmpdir):
assert 'S1145' in cov['JAVASCRIPT']
assert cov['JAVASCRIPT']['S1145'] == {'since': REPO + ' 3.3.0.5702', 'until': REPO + ' 6.7.0.14237'}
@patch('rspec_tools.coverage.REPOS', ['SonarJS', 'sonar-xml'])
def test_update_coverage_for_all_repos(tmpdir):
with pushd(tmpdir):

View File

@ -1,215 +1,33 @@
import json
import os
import shutil
from pathlib import Path
from typing import Optional
from unittest.mock import Mock, patch
import pytest
from git import Head, Repo
from git import Repo
from rspec_tools.create_rule import (RuleCreator, add_language_to_rule,
create_new_rule,
update_rule_quickfix_status)
create_new_rule)
from rspec_tools.errors import InvalidArgumentError
from rspec_tools.repo import RspecRepo
from rspec_tools.utils import LANG_TO_SOURCE, is_empty_metadata
from tests.conftest import mock_github
@pytest.fixture
def git_config():
'''Create a mock git configuration.'''
return {
'user.name': 'testuser',
'user.email': 'testuser@mock.mock'
}
@pytest.fixture
def mock_rspec_repo(tmpdir, mockrules: Path):
repo_dir = tmpdir.mkdir("mock_rspec")
repo = Repo.init(str(repo_dir))
repo.init()
rules_dir = repo_dir.join('rules')
shutil.copytree(mockrules, rules_dir)
with repo.config_writer() as config_writer:
config_writer.set_value('user', 'name', 'originuser')
config_writer.set_value('user', 'email', 'originuser@mock.mock')
repo.git.add('--all')
repo.index.commit('init rules')
# Create the id counter branch. Note that it is an orphan branch.
repo.head.reference = Head(repo, f'refs/heads/{RuleCreator.ID_COUNTER_BRANCH}')
repo.git.reset('--hard')
counter_file = repo_dir.join(RuleCreator.ID_COUNTER_FILENAME)
counter_file.write('0')
repo.index.add([str(counter_file)])
commit = repo.index.commit('init counter', parent_commits=None)
# Checkout a specific commit so that the repo can be pushed to without
# making the index and work tree inconsistent.
repo.git.checkout(commit.hexsha)
return repo
@pytest.fixture
def rule_creator(tmpdir, mock_rspec_repo: Repo, git_config: dict[str, str]):
cloned_repo = tmpdir.mkdir("cloned_repo")
return RuleCreator(mock_rspec_repo.working_dir, str(cloned_repo), git_config)
def rule_creator(rspec_repo: RspecRepo):
return RuleCreator(rspec_repo)
def test_reserve_rule_number_simple(rule_creator: RuleCreator, mock_rspec_repo: Repo):
'''Test that RuleCreator.reserve_rule_id() increments the id and returns the old value.'''
assert rule_creator.reserve_rule_number() == 0
assert read_counter_file(mock_rspec_repo) == '1'
def test_reserve_rule_number_parallel_reservations(tmpdir, mock_rspec_repo: Repo, git_config):
'''Test that RuleCreator.reserve_rule_id() works when multiple reservations are done in parallel.'''
cloned_repo1 = tmpdir.mkdir("cloned_repo1")
rule_creator1 = RuleCreator(mock_rspec_repo.working_dir, str(cloned_repo1), git_config)
cloned_repo2 = tmpdir.mkdir("cloned_repo2")
rule_creator2 = RuleCreator(mock_rspec_repo.working_dir, str(cloned_repo2), git_config)
assert rule_creator1.reserve_rule_number() == 0
assert rule_creator2.reserve_rule_number() == 1
assert rule_creator1.reserve_rule_number() == 2
assert read_counter_file(mock_rspec_repo) == '3'
def read_counter_file(repo):
'''Reads the counter file from the provided repository and returns its content.'''
repo.git.checkout(RuleCreator.ID_COUNTER_BRANCH)
counter_path = Path(repo.working_dir).joinpath(RuleCreator.ID_COUNTER_FILENAME)
return counter_path.read_text()
def test_update_quickfix_status_branch1(rule_creator: RuleCreator, mock_rspec_repo: Repo):
'''Test update_quickfix_status_branch when quickfix field is not present in language-specific metadata'''
rule_number = 100
language = 'cfamily'
sub_path = Path('rules', f'S{rule_number}', language, 'metadata.json')
metadata_path = Path(mock_rspec_repo.working_dir) / sub_path
mock_rspec_repo.git.checkout('master')
initial_metadata = json.loads(metadata_path.read_text())
assert 'quickfix' not in initial_metadata # It is in the parent metadata.json file.
branch = rule_creator.update_quickfix_status_branch('Some title', rule_number, language, 'covered')
mock_rspec_repo.git.checkout(branch)
new_metadata = json.loads(metadata_path.read_text())
# Verify that only the quickfix status is introduced.
assert len(initial_metadata.keys()) + 1 == len(new_metadata.keys())
assert 'quickfix' in new_metadata
assert 'covered' == new_metadata['quickfix']
for key in initial_metadata:
assert initial_metadata[key] == new_metadata[key]
# Ensure only one file is modified.
modified_files = mock_rspec_repo.git.diff('master', '--name-only').strip()
assert modified_files == str(sub_path)
def test_update_quickfix_status_branch2(rule_creator: RuleCreator, mock_rspec_repo: Repo):
'''Test update_quickfix_status_branch when quickfix field is already present in language-specific metadata'''
rule_number = 100
language = 'java'
sub_path = Path('rules', f'S{rule_number}', language, 'metadata.json')
metadata_path = Path(mock_rspec_repo.working_dir) / sub_path
mock_rspec_repo.git.checkout('master')
initial_metadata = json.loads(metadata_path.read_text())
assert 'quickfix' in initial_metadata
assert initial_metadata['quickfix'] == 'targeted'
branch = rule_creator.update_quickfix_status_branch('Some title', rule_number, language, 'covered')
mock_rspec_repo.git.checkout(branch)
new_metadata = json.loads(metadata_path.read_text())
# Verify that only the quickfix status is updated.
assert len(initial_metadata.keys()) == len(new_metadata.keys())
assert 'quickfix' in new_metadata
assert 'covered' == new_metadata['quickfix']
for key in initial_metadata:
if key != 'quickfix':
assert initial_metadata[key] == new_metadata[key]
# Ensure only one file is modified.
modified_files = mock_rspec_repo.git.diff('master', '--name-only').strip()
assert modified_files == str(sub_path)
def test_update_quickfix_status_branch3(rule_creator: RuleCreator, mock_rspec_repo: Repo):
'''Test update_quickfix_status_branch when new and old quickfix status are the same'''
rule_number = 100
language = 'java'
metadata_path = Path(mock_rspec_repo.working_dir, 'rules', f'S{rule_number}', language, 'metadata.json')
mock_rspec_repo.git.checkout('master')
initial_metadata = json.loads(metadata_path.read_text())
assert 'quickfix' in initial_metadata
assert initial_metadata['quickfix'] == 'targeted'
with pytest.raises(InvalidArgumentError):
rule_creator.update_quickfix_status_branch('Some title', rule_number, language, 'targeted')
def test_update_quickfix_status_branch4(rule_creator: RuleCreator, mock_rspec_repo: Repo):
'''Test update_quickfix_status_branch when the rule does not exist'''
rule_number = 404
language = 'java'
metadata_path = Path(mock_rspec_repo.working_dir, 'rules', f'S{rule_number}', language, 'metadata.json')
mock_rspec_repo.git.checkout('master')
assert not metadata_path.exists()
with pytest.raises(InvalidArgumentError):
rule_creator.update_quickfix_status_branch('Some title', rule_number, language, 'targeted')
def test_update_quickfix_status_pull_request(rule_creator: RuleCreator):
'''Test update_quickfix_status_pull_request adds the right user and label.'''
ghMock = Mock()
ghRepoMock = Mock()
pullMock = Mock()
ghRepoMock.create_pull = Mock(return_value=pullMock)
ghMock.get_repo = Mock(return_value=ghRepoMock)
def mockGithub(user: Optional[str]):
return ghMock
rule_creator.update_quickfix_status_pull_request(mockGithub, 100, 'cfamily', 'covered', 'label-fraicheur', 'testuser')
ghRepoMock.create_pull.assert_called_once();
assert ghRepoMock.create_pull.call_args.kwargs['title'].startswith('Modify rule S100')
pullMock.add_to_assignees.assert_called_with('testuser')
pullMock.add_to_labels.assert_called_with('label-fraicheur')
@patch('rspec_tools.create_rule.RuleCreator')
def test_update_rule_quickfix_status(mockRuleCreator):
'''Test update_rule_quickfix_status uses the expected implementation.'''
mockRuleCreator.return_value = Mock()
mockRuleCreator.return_value.update_quickfix_status_pull_request = Mock()
prMock = mockRuleCreator.return_value.update_quickfix_status_pull_request
update_rule_quickfix_status('cfamily', 'S100', 'covered', 'my token', 'testuser')
prMock.assert_called_once()
assert prMock.call_args.args[1] == 100
assert prMock.call_args.args[2] == 'cfamily'
assert prMock.call_args.args[3] == 'covered'
assert prMock.call_args.args[4] == 'cfamily'
def test_create_new_multi_lang_rule_branch(rule_creator: RuleCreator, mock_rspec_repo: Repo):
def test_create_new_multi_lang_rule_branch(rule_creator: RuleCreator, mock_git_rspec_repo: Repo):
'''Test create_new_rule_branch for a multi-language rule.'''
rule_number = rule_creator.reserve_rule_number()
rule_number = rule_creator.rspec_repo.reserve_rule_number()
languages = ['java', 'javascript']
branch = rule_creator.create_new_rule_branch(rule_number, languages)
# Check that the branch was pushed successfully to the origin
mock_rspec_repo.git.checkout(branch)
rule_dir = Path(mock_rspec_repo.working_dir).joinpath('rules', f'S{rule_number}')
mock_git_rspec_repo.git.checkout(branch)
rule_dir = Path(mock_git_rspec_repo.working_dir).joinpath('rules', f'S{rule_number}')
assert rule_dir.exists()
common_root = rule_creator.TEMPLATE_PATH.joinpath('multi_language', 'common')
@ -230,16 +48,17 @@ def test_create_new_multi_lang_rule_branch(rule_creator: RuleCreator, mock_rspec
actual_content = rule_dir.joinpath(lang, relative_path).read_text()
assert actual_content == expected_content
def test_create_new_single_lang_rule_branch(rule_creator: RuleCreator, mock_rspec_repo: Repo):
def test_create_new_single_lang_rule_branch(rule_creator: RuleCreator, mock_git_rspec_repo: Repo):
'''Test create_new_rule_branch for a single-language rule.'''
rule_number = rule_creator.reserve_rule_number()
rule_number = rule_creator.rspec_repo.reserve_rule_number()
languages = ['cfamily']
branch = rule_creator.create_new_rule_branch(rule_number, languages)
# Check that the branch was pushed successfully to the origin
mock_rspec_repo.git.checkout(branch)
rule_dir = Path(mock_rspec_repo.working_dir).joinpath('rules', f'S{rule_number}')
mock_git_rspec_repo.git.checkout(branch)
rule_dir = Path(mock_git_rspec_repo.working_dir).joinpath('rules', f'S{rule_number}')
assert rule_dir.exists()
common_root = rule_creator.TEMPLATE_PATH.joinpath('single_language', 'common')
@ -261,52 +80,43 @@ def test_create_new_single_lang_rule_branch(rule_creator: RuleCreator, mock_rspe
actual_content = rule_dir.joinpath(lang, relative_path).read_text()
assert actual_content == expected_content
def test_create_new_rule_pr(rule_creator: RuleCreator):
def test_create_new_rule_pull_request(rule_creator: RuleCreator):
'''Test create_new_rule_branch adds the right user and labels.'''
rule_number = rule_creator.reserve_rule_number()
rule_number = rule_creator.rspec_repo.reserve_rule_number()
languages = ['cfamily']
ghMock = Mock()
ghRepoMock = Mock()
pullMock = Mock()
ghRepoMock.create_pull = Mock(return_value=pullMock)
ghMock.get_repo = Mock(return_value=ghRepoMock)
def mockGithub(user: Optional[str]):
return ghMock
with mock_github() as (token, user, mock_repo):
rule_creator.create_new_rule_pull_request(token, rule_number, languages, ['mylab', 'other-lab'], user)
rule_creator.create_new_rule_pull_request(mockGithub, rule_number, languages, ['mylab', 'other-lab'], user='testuser')
mock_repo.create_pull.assert_called_once();
assert mock_repo.create_pull.call_args.kwargs['title'].startswith('Create rule S')
mock_repo.create_pull.return_value.add_to_assignees.assert_called_with(user);
mock_repo.create_pull.return_value.add_to_labels.assert_called_with('mylab', 'other-lab');
ghRepoMock.create_pull.assert_called_once();
assert ghRepoMock.create_pull.call_args.kwargs['title'].startswith('Create rule S')
pullMock.add_to_assignees.assert_called_with('testuser');
pullMock.add_to_labels.assert_called_with('mylab', 'other-lab');
@patch('rspec_tools.create_rule.RuleCreator')
def test_create_new_rule(mockRuleCreator):
mockRuleCreator.return_value = Mock()
mockRuleCreator.return_value.create_new_rule_pull_request = Mock()
prMock = mockRuleCreator.return_value.create_new_rule_pull_request
create_new_rule('cfamily,php', 'my token', 'testuser')
prMock.assert_called_once()
assert set(prMock.call_args.args[2]) == set(['cfamily', 'php'])
assert set(prMock.call_args.args[3]) == set(['cfamily', 'php'])
@patch('rspec_tools.create_rule.RuleCreator')
def test_create_new_rule_unsupported_language(mockRuleCreator):
mockRuleCreator.return_value = Mock()
mockRuleCreator.return_value.create_new_rule_pull_request = Mock()
prMock = mockRuleCreator.return_value.create_new_rule_pull_request
with pytest.raises(InvalidArgumentError):
create_new_rule('russian,php', 'my token', 'testuser')
def test_add_lang_singlelang_nonconventional_rule_create_branch(rule_creator: RuleCreator, mock_rspec_repo: Repo):
def test_add_lang_singlelang_nonconventional_rule_create_branch(rule_creator: RuleCreator, mock_git_rspec_repo: Repo):
'''Test add_language_branch for a single-language rule with metadata lifted to the generic rule level.'''
rule_number = 4727
language = 'php'
mock_rspec_repo.git.checkout('master')
orig_rule_dir = Path(mock_rspec_repo.working_dir).joinpath('rules', f'S{rule_number}')
mock_git_rspec_repo.git.checkout('master')
orig_rule_dir = Path(mock_git_rspec_repo.working_dir).joinpath('rules', f'S{rule_number}')
assert(not is_empty_metadata(orig_rule_dir)) # nonconventional: singlelang rule with metadata on the upper level
assert(is_empty_metadata(orig_rule_dir.joinpath('cobol')))
original_metadata = orig_rule_dir.joinpath('metadata.json').read_text()
@ -314,8 +124,8 @@ def test_add_lang_singlelang_nonconventional_rule_create_branch(rule_creator: Ru
branch = rule_creator.add_language_branch(rule_number, language)
# Check that the branch was pushed successfully to the origin
mock_rspec_repo.git.checkout(branch)
rule_dir = Path(mock_rspec_repo.working_dir).joinpath('rules', f'S{rule_number}')
mock_git_rspec_repo.git.checkout(branch)
rule_dir = Path(mock_git_rspec_repo.working_dir).joinpath('rules', f'S{rule_number}')
assert rule_dir.exists()
lang_dir = rule_dir.joinpath(f'{language}')
assert lang_dir.exists()
@ -332,13 +142,14 @@ def test_add_lang_singlelang_nonconventional_rule_create_branch(rule_creator: Ru
actual_content = rule_dir.joinpath(language, relative_path).read_text()
assert actual_content == expected_content
def test_add_lang_singlelang_conventional_rule_create_branch(rule_creator: RuleCreator, mock_rspec_repo: Repo):
def test_add_lang_singlelang_conventional_rule_create_branch(rule_creator: RuleCreator, mock_git_rspec_repo: Repo):
'''Test add_language_branch for a regular single language rule.'''
rule_number = 1033
language = 'php'
mock_rspec_repo.git.checkout('master')
orig_rule_dir = Path(mock_rspec_repo.working_dir).joinpath('rules', f'S{rule_number}')
mock_git_rspec_repo.git.checkout('master')
orig_rule_dir = Path(mock_git_rspec_repo.working_dir).joinpath('rules', f'S{rule_number}')
assert(is_empty_metadata(orig_rule_dir)) # conventional: singlelang rule with metadata on the lang-specific level
assert(not is_empty_metadata(orig_rule_dir.joinpath('cfamily')))
original_lmetadata = orig_rule_dir.joinpath('cfamily', 'metadata.json').read_text()
@ -346,8 +157,8 @@ def test_add_lang_singlelang_conventional_rule_create_branch(rule_creator: RuleC
branch = rule_creator.add_language_branch(rule_number, language)
# Check that the branch was pushed successfully to the origin
mock_rspec_repo.git.checkout(branch)
rule_dir = Path(mock_rspec_repo.working_dir).joinpath('rules', f'S{rule_number}')
mock_git_rspec_repo.git.checkout(branch)
rule_dir = Path(mock_git_rspec_repo.working_dir).joinpath('rules', f'S{rule_number}')
assert rule_dir.exists()
lang_dir = rule_dir.joinpath(f'{language}')
assert lang_dir.exists()
@ -355,7 +166,8 @@ def test_add_lang_singlelang_conventional_rule_create_branch(rule_creator: RuleC
assert rule_dir.joinpath('metadata.json').read_text() == original_lmetadata
assert(is_empty_metadata(rule_dir.joinpath('cfamily')))
def test_add_lang_multilang_rule_create_branch(rule_creator: RuleCreator, mock_rspec_repo: Repo):
def test_add_lang_multilang_rule_create_branch(rule_creator: RuleCreator, mock_git_rspec_repo: Repo):
'''Test add_language_branch for a multi-language rule.'''
rule_number = 120
language = 'php'
@ -363,8 +175,8 @@ def test_add_lang_multilang_rule_create_branch(rule_creator: RuleCreator, mock_r
branch = rule_creator.add_language_branch(rule_number, language)
# Check that the branch was pushed successfully to the origin
mock_rspec_repo.git.checkout(branch)
rule_dir = Path(mock_rspec_repo.working_dir).joinpath('rules', f'S{rule_number}')
mock_git_rspec_repo.git.checkout(branch)
rule_dir = Path(mock_git_rspec_repo.working_dir).joinpath('rules', f'S{rule_number}')
assert rule_dir.exists()
lang_dir = rule_dir.joinpath(f'{language}')
assert lang_dir.exists()
@ -378,42 +190,44 @@ def test_add_lang_multilang_rule_create_branch(rule_creator: RuleCreator, mock_r
actual_content = rule_dir.joinpath(language, relative_path).read_text()
assert actual_content == expected_content
@patch('rspec_tools.create_rule.RuleCreator')
def test_add_unsupported_language(mockRuleCreator):
def test_add_unsupported_language(mock):
'''Test language validation.'''
mockRuleCreator.return_value = Mock()
mockRuleCreator.return_value.create_new_rule_pull_request = Mock()
prMock = mockRuleCreator.return_value.create_new_rule_pull_request
with pytest.raises(InvalidArgumentError):
add_language_to_rule('russian', 'S1033', 'my token', 'testuser')
mock.return_value.add_language_pull_request.assert_not_called()
@patch('rspec_tools.create_rule.RuleCreator')
def test_add_supported_language(mock):
'''Test language validation.'''
add_language_to_rule('cfamily', 'S1033', 'my token', 'testuser')
mock.return_value.add_language_pull_request.assert_called_once()
def test_add_language_the_rule_is_already_defined_for(rule_creator: RuleCreator):
'''Test add_language_branch fails when trying to add a langage already added to the rule.'''
with pytest.raises(InvalidArgumentError):
rule_creator.add_language_branch(100, 'cfamily')
def test_add_language_to_nonexistent_rule(rule_creator: RuleCreator):
'''Test add_language_branch correctly fails when invoked for a non-existent rule.'''
with pytest.raises(InvalidArgumentError):
rule_creator.add_language_branch(101, 'cfamily')
def test_add_language_new_pr(rule_creator: RuleCreator):
def test_add_language_new_pull_request(rule_creator: RuleCreator):
'''Test add_language_pull_request adds the right user and labels.'''
rule_number = 120
language = 'php'
ghMock = Mock()
ghRepoMock = Mock()
pullMock = Mock()
ghRepoMock.create_pull = Mock(return_value=pullMock)
ghMock.get_repo = Mock(return_value=ghRepoMock)
def mockGithub(user: Optional[str]):
return ghMock
with mock_github() as (token, user, mock_repo):
rule_creator.add_language_pull_request(token, rule_number, language, 'mylab', user)
rule_creator.add_language_pull_request(mockGithub, rule_number, language, 'mylab', user='testuser')
ghRepoMock.create_pull.assert_called_once();
assert ghRepoMock.create_pull.call_args.kwargs['title'].startswith(f'Create rule S{rule_number}')
ghRepoMock.create_pull.call_args.kwargs['head'].startswith('rule/')
pullMock.add_to_assignees.assert_called_with('testuser');
pullMock.add_to_labels.assert_called_with('mylab');
mock_repo.create_pull.assert_called_once();
assert mock_repo.create_pull.call_args.kwargs['title'].startswith(f'Create rule S{rule_number}')
assert mock_repo.create_pull.call_args.kwargs['head'].startswith('rule/')
mock_repo.create_pull.return_value.add_to_assignees.assert_called_with(user)
mock_repo.create_pull.return_value.add_to_labels.assert_called_with('mylab')

View File

@ -0,0 +1,122 @@
import json
from pathlib import Path
from unittest.mock import Mock, patch
import pytest
from git import Repo
from rspec_tools.errors import InvalidArgumentError
from rspec_tools.modify_rule import RuleEditor, update_rule_quickfix_status
from rspec_tools.repo import RspecRepo
from tests.conftest import mock_github
@pytest.fixture
def rule_editor(rspec_repo: RspecRepo):
return RuleEditor(rspec_repo)
def test_update_quickfix_status_branch1(rule_editor: RuleEditor, mock_git_rspec_repo: Repo):
'''Test update_quickfix_status_branch when quickfix field is not present in language-specific metadata'''
rule_number = 100
language = 'cfamily'
sub_path = Path('rules', f'S{rule_number}', language, 'metadata.json')
metadata_path = Path(mock_git_rspec_repo.working_dir) / sub_path
mock_git_rspec_repo.git.checkout('master')
initial_metadata = json.loads(metadata_path.read_text())
assert 'quickfix' not in initial_metadata # It is in the parent metadata.json file.
branch = rule_editor.update_quickfix_status_branch('Some title', rule_number, language, 'covered')
mock_git_rspec_repo.git.checkout(branch)
new_metadata = json.loads(metadata_path.read_text())
# Verify that only the quickfix status is introduced.
assert len(initial_metadata.keys()) + 1 == len(new_metadata.keys())
assert 'quickfix' in new_metadata
assert 'covered' == new_metadata['quickfix']
for key in initial_metadata:
assert initial_metadata[key] == new_metadata[key]
# Ensure only one file is modified.
modified_files = mock_git_rspec_repo.git.diff('master', '--name-only').strip()
assert modified_files == str(sub_path)
def test_update_quickfix_status_branch2(rule_editor: RuleEditor, mock_git_rspec_repo: Repo):
'''Test update_quickfix_status_branch when quickfix field is already present in language-specific metadata'''
rule_number = 100
language = 'java'
sub_path = Path('rules', f'S{rule_number}', language, 'metadata.json')
metadata_path = Path(mock_git_rspec_repo.working_dir) / sub_path
mock_git_rspec_repo.git.checkout('master')
initial_metadata = json.loads(metadata_path.read_text())
assert 'quickfix' in initial_metadata
assert initial_metadata['quickfix'] == 'targeted'
branch = rule_editor.update_quickfix_status_branch('Some title', rule_number, language, 'covered')
mock_git_rspec_repo.git.checkout(branch)
new_metadata = json.loads(metadata_path.read_text())
# Verify that only the quickfix status is updated.
assert len(initial_metadata.keys()) == len(new_metadata.keys())
assert 'quickfix' in new_metadata
assert 'covered' == new_metadata['quickfix']
for key in initial_metadata:
if key != 'quickfix':
assert initial_metadata[key] == new_metadata[key]
# Ensure only one file is modified.
modified_files = mock_git_rspec_repo.git.diff('master', '--name-only').strip()
assert modified_files == str(sub_path)
def test_update_quickfix_status_branch3(rule_editor: RuleEditor, mock_git_rspec_repo: Repo):
'''Test update_quickfix_status_branch when new and old quickfix status are the same'''
rule_number = 100
language = 'java'
metadata_path = Path(mock_git_rspec_repo.working_dir, 'rules', f'S{rule_number}', language, 'metadata.json')
mock_git_rspec_repo.git.checkout('master')
initial_metadata = json.loads(metadata_path.read_text())
assert 'quickfix' in initial_metadata
assert initial_metadata['quickfix'] == 'targeted'
with pytest.raises(InvalidArgumentError):
rule_editor.update_quickfix_status_branch('Some title', rule_number, language, 'targeted')
def test_update_quickfix_status_branch4(rule_editor: RuleEditor, mock_git_rspec_repo: Repo):
'''Test update_quickfix_status_branch when the rule does not exist'''
rule_number = 404
language = 'java'
metadata_path = Path(mock_git_rspec_repo.working_dir, 'rules', f'S{rule_number}', language, 'metadata.json')
mock_git_rspec_repo.git.checkout('master')
assert not metadata_path.exists()
with pytest.raises(InvalidArgumentError):
rule_editor.update_quickfix_status_branch('Some title', rule_number, language, 'targeted')
def test_update_quickfix_status_pull_request(rule_editor: RuleEditor):
'''Test update_quickfix_status_pull_request adds the right user and label.'''
with mock_github() as (token, user, mock_repo):
rule_editor.update_quickfix_status_pull_request(token, 100, 'cfamily', 'covered', 'label-fraicheur', user)
mock_repo.create_pull.assert_called_once();
assert mock_repo.create_pull.call_args.kwargs['title'].startswith('Modify rule S100')
mock_repo.create_pull.return_value.add_to_assignees.assert_called_with(user)
mock_repo.create_pull.return_value.add_to_labels.assert_called_with('label-fraicheur')
@patch('rspec_tools.modify_rule.RuleEditor')
def test_update_rule_quickfix_status(mockRuleEditor):
'''Test update_rule_quickfix_status uses the expected implementation.'''
prMock = mockRuleEditor.return_value.update_quickfix_status_pull_request
update_rule_quickfix_status('cfamily', 'S100', 'covered', 'my token', 'testuser')
prMock.assert_called_once()
assert prMock.call_args.args[1] == 100
assert prMock.call_args.args[2] == 'cfamily'
assert prMock.call_args.args[3] == 'covered'
assert prMock.call_args.args[4] == 'cfamily'

View File

@ -0,0 +1,33 @@
from pathlib import Path
import pytest
from git import Repo
from rspec_tools.repo import RspecRepo
def _read_counter_file(repo: Repo):
'''Reads the counter file from the provided repository and returns its content.'''
repo.git.checkout(RspecRepo.ID_COUNTER_BRANCH)
counter_path = Path(repo.working_dir, RspecRepo.ID_COUNTER_FILENAME)
return counter_path.read_text()
def test_reserve_rule_number_simple(rspec_repo: RspecRepo, mock_git_rspec_repo: Repo):
'''Test that RspecRepo.reserve_rule_number() increments the id and returns the old value.'''
assert rspec_repo.reserve_rule_number() == 0
assert _read_counter_file(mock_git_rspec_repo) == '1'
def test_reserve_rule_number_parallel_reservations(tmpdir, mock_git_rspec_repo: Repo, git_config):
'''Test that RspecRepo.reserve_rule_number() works when multiple reservations are done in parallel.'''
cloned_repo1 = tmpdir.mkdir("cloned_repo1")
rule_creator1 = RspecRepo(mock_git_rspec_repo.working_dir, str(cloned_repo1), git_config)
cloned_repo2 = tmpdir.mkdir("cloned_repo2")
rule_creator2 = RspecRepo(mock_git_rspec_repo.working_dir, str(cloned_repo2), git_config)
assert rule_creator1.reserve_rule_number() == 0
assert rule_creator2.reserve_rule_number() == 1
assert rule_creator1.reserve_rule_number() == 2
assert _read_counter_file(mock_git_rspec_repo) == '3'

View File

@ -1,8 +1,5 @@
from pathlib import Path
import pytest
from pytest import TempPathFactory
from rspec_tools.rules import RulesRepository
def test_list_rules(mockrules: Path):

View File

@ -1,6 +1,10 @@
from rspec_tools.errors import InvalidArgumentError
from rspec_tools.utils import parse_and_validate_language_list, load_valid_languages, get_mapped_languages, get_labels_for_languages, resolve_rule, validate_language, get_label_for_language
import pytest
from rspec_tools.errors import InvalidArgumentError
from rspec_tools.utils import (get_label_for_language,
get_labels_for_languages, get_mapped_languages,
load_valid_languages,
parse_and_validate_language_list, resolve_rule)
def test_fails_when_no_languages_listed():
'''Test that language validation fails on empty list.'''
@ -52,8 +56,5 @@ def test_resolve_rule():
def test_label_for_language():
assert get_label_for_language('java') == 'java'
def test_validate_language():
validate_language('java')
with pytest.raises(InvalidArgumentError):
validate_language('russian')
get_label_for_language('russian')