Add support for a custom path root separator using a trait. (#3678)
* - Add support for a custom path root separator using a trait. - Ensure to set both FilePathKey_ProjectUserPath and FilePathKey_ProjectLogPath to a writeable storage location on non-host platforms. Signed-off-by: bosnichd <bosnichd@amazon.com> * Remove az_trait_validator.py, as our PAL guidance no longer dictates that all traits must be defined for all platforms. Signed-off-by: bosnichd <bosnichd@amazon.com> * Updated based on review feedback. Signed-off-by: bosnichd <bosnichd@amazon.com>
This commit is contained in:
bosnichd
GitHub
-92
@@ -1,92 +0,0 @@
|
||||
#
|
||||
# Copyright (c) Contributors to the Open 3D Engine Project.
|
||||
# For complete copyright and license terms please see the LICENSE at the root of this distribution.
|
||||
#
|
||||
# SPDX-License-Identifier: Apache-2.0 OR MIT
|
||||
#
|
||||
#
|
||||
|
||||
import unittest
|
||||
from unittest.mock import patch, mock_open
|
||||
|
||||
from commit_validation import pal_allowedlist
|
||||
from commit_validation.tests.mocks.mock_commit import MockCommit
|
||||
from commit_validation.validators.az_trait_validator import AzTraitValidator
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
class Test_AzTraitValidatorTests():
|
||||
def test_fileDoesntCheckAzTraitIsDefined_passes(self):
|
||||
commit = MockCommit(
|
||||
files=['someCppFile.cpp'],
|
||||
file_diffs={ 'someCppFile.cpp' : ''}
|
||||
)
|
||||
error_list = []
|
||||
assert AzTraitValidator().run(commit, error_list)
|
||||
assert len(error_list) == 0, f"Unexpected errors: {error_list}"
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
'file_diffs,expect_success', [
|
||||
pytest.param('+This file does contain\n'
|
||||
'+a trait existence check\n'
|
||||
'+#ifdef AZ_TRAIT_USED_INCORRECTLY\n',
|
||||
False,
|
||||
id="AZ_TRAIT_inside_ifdef_fails" ), # gives the test a friendly name!
|
||||
|
||||
pytest.param('+This file does contain\n'
|
||||
'+a trait existence check\n'
|
||||
'+#if defined(AZ_TRAIT_USED_INCORRECTLY)\n',
|
||||
False,
|
||||
id="AZ_TRAIT_inside_if_defined_fails" ),
|
||||
|
||||
pytest.param('+This file does contain\n'
|
||||
'+a trait existence check\n'
|
||||
'+#ifndef AZ_TRAIT_USED_INCORRECTLY\n',
|
||||
False,
|
||||
id="AZ_TRAIT_inside_ifndef_fails" ),
|
||||
|
||||
pytest.param('+This file contains a diff which REMOVES an incorrect usage\n'
|
||||
'-#ifndef AZ_TRAIT_USED_INCORRECTLY\n',
|
||||
True,
|
||||
id="AZ_TRAIT_removed_in_diff_passes" ),
|
||||
|
||||
pytest.param('+This file contains a diff which has an old already okayed usage\n'
|
||||
'+which is not actually part of the diff.\n'
|
||||
'#ifndef AZ_TRAIT_USED_INCORRECTLY\n',
|
||||
True,
|
||||
id="AZ_TRAIT_in_unmodified_section_passes"),
|
||||
|
||||
pytest.param('+This file contains the correct usage\n'
|
||||
'+#if AZ_TRAIT_USED_CORRECTLY\n',
|
||||
True,
|
||||
id="AZ_TRAIT_correct_usage_passes"),
|
||||
])
|
||||
def test_fileChecksAzTraitIsDefined(self, file_diffs, expect_success):
|
||||
commit = MockCommit(
|
||||
files=['someCppFile.cpp'],
|
||||
file_diffs={ 'someCppFile.cpp' : file_diffs })
|
||||
|
||||
error_list = []
|
||||
if expect_success:
|
||||
assert AzTraitValidator().run(commit, error_list)
|
||||
assert len(error_list) == 0, f"Unexpected errors: {error_list}"
|
||||
else:
|
||||
assert not AzTraitValidator().run(commit, error_list)
|
||||
assert len(error_list) != 0, f"Errors were expected but none were returned."
|
||||
|
||||
def test_fileExtensionIgnored_passes(self):
|
||||
commit = MockCommit(files=['someCppFile.waf_files'])
|
||||
error_list = []
|
||||
assert AzTraitValidator().run(commit, error_list)
|
||||
assert len(error_list) == 0, f"Unexpected errors: {error_list}"
|
||||
|
||||
@patch('commit_validation.pal_allowedlist.load', return_value=pal_allowedlist.PALAllowedlist(['*/some/path/*']))
|
||||
def test_fileAllowedlisted_passes(self, mocked_load):
|
||||
commit = MockCommit(files=['/path/to/some/path/someCppFile.cpp'])
|
||||
error_list = []
|
||||
assert AzTraitValidator().run(commit, error_list)
|
||||
assert len(error_list) == 0, f"Unexpected errors: {error_list}"
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
@@ -1,57 +0,0 @@
|
||||
#
|
||||
# Copyright (c) Contributors to the Open 3D Engine Project.
|
||||
# For complete copyright and license terms please see the LICENSE at the root of this distribution.
|
||||
#
|
||||
# SPDX-License-Identifier: Apache-2.0 OR MIT
|
||||
#
|
||||
#
|
||||
|
||||
import os
|
||||
import re
|
||||
from typing import Type, List
|
||||
|
||||
import commit_validation.pal_allowedlist as pal_allowedlist
|
||||
from commit_validation.commit_validation import Commit, CommitValidator, SOURCE_FILE_EXTENSIONS, VERBOSE
|
||||
|
||||
ifdef_regex = re.compile(r'^\+\s*#\s*ifn?def\s+AZ_TRAIT_')
|
||||
defined_regex = re.compile(r'\sdefined\s*\(\s*AZ_TRAIT_')
|
||||
|
||||
|
||||
class AzTraitValidator(CommitValidator):
|
||||
"""A file-level validator that makes sure a file does not contain existence checks for AZ_TRAIT macros"""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self.pal_allowedlist = pal_allowedlist.load()
|
||||
|
||||
def run(self, commit: Commit, errors: List[str]) -> bool:
|
||||
for file_name in commit.get_files():
|
||||
if os.path.splitext(file_name)[1].lower() not in SOURCE_FILE_EXTENSIONS:
|
||||
if VERBOSE: print(f'{file_name}::{self.__class__.__name__} SKIPPED - File excluded based on extension.')
|
||||
continue
|
||||
if self.pal_allowedlist.is_match(file_name):
|
||||
if VERBOSE: print(f'{file_name}::{self.__class__.__name__} SKIPPED - File excluded based on PAL allowedlist.')
|
||||
continue
|
||||
|
||||
file_diff = commit.get_file_diff(file_name)
|
||||
previous_line_context = ""
|
||||
|
||||
for line in file_diff.splitlines():
|
||||
# we only care about added lines.
|
||||
if line.startswith('+'):
|
||||
if ifdef_regex.search(line) or defined_regex.search(line):
|
||||
error_message = str(
|
||||
f'{file_name}::{self.__class__.__name__} FAILED - Source file contains an existence '
|
||||
f'check for an AZ_TRAIT macro in this code: \n'
|
||||
f' {previous_line_context}\n'
|
||||
f' ----> {line}\n'
|
||||
f'Traits should be tested for true/false, since they are guaranteed to exist on all platforms.')
|
||||
if VERBOSE: print(error_message)
|
||||
errors.append(error_message)
|
||||
previous_line_context = line
|
||||
|
||||
return (not errors)
|
||||
|
||||
|
||||
def get_validator() -> Type[AzTraitValidator]:
|
||||
"""Returns the validator class for this module"""
|
||||
return AzTraitValidator
|
||||
Reference in New Issue
Block a user