Skip to content

Commit

Permalink
Code Quality Reports CI Template Follow-Up (Azure#26221)
Browse files Browse the repository at this point in the history
Code Quality Reports CI Template Follow-Up
  • Loading branch information
alzimmermsft authored Jan 4, 2022
1 parent 3c75676 commit 79ba9a0
Show file tree
Hide file tree
Showing 15 changed files with 77 additions and 49 deletions.
2 changes: 2 additions & 0 deletions eng/code-quality-reports/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ trigger:
paths:
include:
- eng/code-quality-reports/
- eng/pipelines/code-quality-reports.yml

pr:
branches:
Expand All @@ -20,6 +21,7 @@ pr:
paths:
include:
- eng/code-quality-reports/
- eng/pipelines/code-quality-reports.yml

extends:
template: /eng/pipelines/code-quality-reports.yml
20 changes: 10 additions & 10 deletions eng/pipelines/code-quality-reports.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
- '!sdk/**/test-recordings'
- '!sdk/**/session-records'

- template: /eng/pipelines/steps/install-reporting-tools.yml
- template: /eng/pipelines/templates/steps/install-reporting-tools.yml

- task: UsePythonVersion@0
displayName: 'Use Python 3.6'
Expand All @@ -39,7 +39,7 @@ jobs:
scriptPath: 'eng/versioning/update_versions.py'
arguments: '--update-type library --build-type client --sr'

- template: /eng/pipelines/steps/generate-project-list.yml
- template: /eng/pipelines/templates/steps/generate-project-list.yml
parameters:
Artifacts:
- name: azure-core
Expand All @@ -56,7 +56,7 @@ jobs:
displayName: 'Generate directories variable for sparse checkout'
inputs:
scriptPath: 'eng/scripts/generate_from_source_pom.py'
arguments: '--set-pipeline-variable CheckoutDirectories --project-list $(ProjectList)'
arguments: '--set-pipeline-variable CheckoutDirectories --set-skip-linting-projects SkipLintingProjects --project-list $(ProjectList)'
workingDirectory: '$(System.DefaultWorkingDirectory)'

- template: /eng/common/pipelines/templates/steps/sparse-checkout.yml
Expand All @@ -69,7 +69,7 @@ jobs:
inputs:
mavenPomFile: ClientFromSourcePom.xml
goals: 'install'
options: '$(DefaultOptions) -T 2C -DskipTests -Dgpg.skip -DtrimStackTrace=false -Dmaven.javadoc.skip=true -Dcodesnippet.skip=true -Dcheckstyle.skip -Dspotbugs.skip -Drevapi.skip=true -Djacoco.skip=true'
options: '$(DefaultOptions) -T 2C -DskipTests -Dgpg.skip -DtrimStackTrace=false -Dmaven.javadoc.skip=true -Dcodesnippet.skip=true -Dcheckstyle.skip -Dspotbugs.skip -Drevapi.skip=true -Djacoco.skip=true --quiet'
mavenOptions: '$(MemoryOptions) $(LoggingOptions)'
javaHomeOption: 'JDKVersion'
jdkVersionOption: $(JavaBuildVersion)
Expand All @@ -80,20 +80,20 @@ jobs:
displayName: 'Run Linting'
inputs:
mavenPomFile: ClientFromSourcePom.xml
goals: 'checkstyle:check spotbugs:check revapi:check'
# Skip failing on Checkstyle, RevApi, and Spotbugs errors.
goals: 'checkstyle:check spotbugs:check'
# Skip failing on Checkstyle and Spotbugs errors.
# This is being done as the error reporting will be captured, so if any error reports exist the build can be
# failed later allowing all libraries to be validated.
options: '$(DefaultOptions) -T 2C -Dcheckstyle.failOnViolation=false -Dcheckstyle.failsOnError=false -Dspotbugs.failOnError=false -Drevapi.failBuildOnProblemsFound=false'
options: '$(DefaultOptions) -T 1C -Dcheckstyle.failOnViolation=false -Dcheckstyle.failsOnError=false -Dspotbugs.failOnError=false -pl "!com.azure:azure-sdk-from-source,$(SkipLintingProjects)"'
mavenOptions: '$(MemoryOptions) $(LoggingOptions)'
javaHomeOption: 'JDKVersion'
jdkVersionOption: $(JavaBuildVersion)
jdkArchitectureOption: 'x64'
publishJUnitResults: false

- task: Powershell@2
- task: PowerShell@2
inputs:
filePath: /eng/pipelines/scripts/Get-Linting-Reports.ps1
filePath: $(System.DefaultWorkingDirectory)/eng/pipelines/scripts/Get-Linting-Reports.ps1
arguments: -OutputDirectory $(Build.ArtifactStagingDirectory)/linting-reports
pwsh: true
workingDirectory: $(Pipeline.Workspace)
Expand All @@ -106,7 +106,7 @@ jobs:
condition: always()

- pwsh: |
if ((Get-ChildItem -Directory | Measure-Object).Count -gt 0) {
if ((Get-ChildItem -Path $(Build.ArtifactStagingDirectory)/linting-reports -Directory | Measure-Object).Count -gt 0) {
exit 1
}
displayName: 'Fail Pipeline if Linting Reports Exist'
3 changes: 2 additions & 1 deletion eng/pipelines/scripts/Get-Linting-Reports.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ function WriteSpotbugsProcessedReport($SpotbugsXmlReport, $ReportOutputFolder) {
Set-Location -ErrorAction Stop -LiteralPath (Join-Path $PSScriptRoot "../../../")

# Always create the output directory
$OutputDirectory = $ExecutionContext.SessionState.Path.GetUnresolvedProviderPathFromPSPath($OutputDirectory)
New-Item -Path $OutputDirectory -ItemType Directory | Out-Null

$path = "sdk/*/"
Expand All @@ -125,7 +126,7 @@ foreach ($targetFolder in (Get-ChildItem -Path $path -Filter "target" -Directory

$spotbugsXmlReport = Join-Path $targetFolder.FullName "spotbugs.xml"
if (Test-Path -Path $spotbugsXmlReport) {
WriteSpotbugsProcessedReport $spotbugsXmlReport $OutputDirectory
WriteSpotbugsProcessedReport $spotbugsXmlReport $reportOutputFolder
}

$revapiReport = Join-Path $targetFolder.FullName "revapi.json"
Expand Down
58 changes: 40 additions & 18 deletions eng/scripts/generate_from_source_pom.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@
import xml.etree.ElementTree as ET

class Project:
def __init__(self, identifier: str, module_path: str, parent_pom: str):
def __init__(self, identifier: str, directory_path: str, module_path: str, parent_pom: str):
self.identifier = identifier
self.directory_path = directory_path
self.module_path = module_path
self.parent_pom = parent_pom
self.dependencies: List[str] = []
Expand All @@ -41,7 +42,7 @@ def add_dependent(self, dependent: str):
if dependent not in self.dependents:
self.dependents.append(dependent)

default_project = Project(None, None, None)
default_project = Project(None, None, None, None)

# azure-client-sdk-parent, azure-perf-test-parent, spring-boot-starter-parent, and azure-spring-boot-test-parent are
# valid parent POMs for Track 2 libraries.
Expand Down Expand Up @@ -79,7 +80,7 @@ def add_dependent(self, dependent: str):
maven_xml_namespace = '{http://maven.apache.org/POM/4.0.0}'

# Function that creates the aggregate POM.
def create_from_source_pom(project_list: str, set_pipeline_variable: str):
def create_from_source_pom(project_list: str, set_pipeline_variable: str, set_skip_linting_projects: str):
project_list_identifiers = project_list.split(',')

# Get the artifact identifiers from client_versions.txt to act as our source of truth.
Expand All @@ -101,14 +102,14 @@ def create_from_source_pom(project_list: str, set_pipeline_variable: str):
for project_identifier in dependent_modules:
dependency_modules = resolve_project_dependencies(project_identifier, dependency_modules, projects)

modules: Set[str] = set()
source_projects: Set[Project] = set()

# Finally map the project identifiers to relative module paths.
add_module_paths(modules, project_list_identifiers, projects)
add_module_paths(modules, dependent_modules, projects)
add_module_paths(modules, dependency_modules, projects)

modules = sorted(modules)
# Finally map the project identifiers to projects.
add_source_projects(source_projects, project_list_identifiers, projects)
add_source_projects(source_projects, dependent_modules, projects)
add_source_projects(source_projects, dependency_modules, projects)
modules = list(set(sorted([p.module_path for p in source_projects])))
with open(file=client_from_source_pom_path, mode='w') as fromSourcePom:
fromSourcePom.write(pom_file_start)

Expand All @@ -118,7 +119,18 @@ def create_from_source_pom(project_list: str, set_pipeline_variable: str):
fromSourcePom.write(pom_file_end)

if set_pipeline_variable:
print('##vso[task.setvariable variable={};]{}'.format(set_pipeline_variable, json.dumps(modules)))
checkout_paths = list(set(sorted([p.directory_path for p in source_projects])))
print('##vso[task.setvariable variable={};]{}'.format(set_pipeline_variable, json.dumps(checkout_paths)))

# Sets the DevOps variable that is used to skip certain projects during linting validation.
if set_skip_linting_projects:
skip_linting_projects = []
for maven_identifier in sorted([p.identifier for p in source_projects]):
if not project_uses_client_parent(projects.get(maven_identifier), projects):
skip_linting_projects.append('!' + maven_identifier)

print('##vso[task.setvariable variable={};]{}'.format(set_skip_linting_projects, ','.join(list(set(skip_linting_projects)))))


# Function that loads and parses client_versions.txt into a artifact identifier - source version mapping.
def load_client_artifact_identifiers() -> Dict[str, str]:
Expand Down Expand Up @@ -174,18 +186,19 @@ def create_project_for_pom(pom_path: str, project_list_identifiers: list, artifa
tree_root = tree.getroot()

project_identifier = create_artifact_identifier(tree_root)
module_path = os.path.dirname(pom_path).replace(root_path, '').replace('\\', '/')
module_path = pom_path.replace(root_path, '').replace('\\', '/')
directory_path = module_path[:module_path.rindex('/')]
parent_pom = get_parent_pom(tree_root)

# If this is one of the parent POMs, retain it as a project.
if project_identifier in parent_pom_identifiers:
return Project(project_identifier, module_path, parent_pom)
return Project(project_identifier, directory_path, module_path, parent_pom)

# If the project isn't a track 2 POM skip it and not one of the project list identifiers.
if not project_identifier in project_list_identifiers and not parent_pom in valid_parents:
return

project = Project(project_identifier, module_path, parent_pom)
project = Project(project_identifier, directory_path, module_path, parent_pom)

dependencies = {child:parent for parent in tree_root.iter() for child in parent if child.tag == maven_xml_namespace + 'dependency'}

Expand Down Expand Up @@ -262,25 +275,34 @@ def get_dependency_version(element: ET.Element):
def element_find(element: ET.Element, path: str):
return element.find(maven_xml_namespace + path)

def add_module_paths(module_paths: Set[str], project_identifiers: Iterable[str], projects: Dict[str, Project]):
def add_source_projects(source_projects: Set[Project], project_identifiers: Iterable[str], projects: Dict[str, Project]):
for project_identifier in project_identifiers:
project = projects[project_identifier]
module_paths.add(project.module_path)
source_projects.add(project)

while project.parent_pom is not None:
project = projects.get(project.parent_pom, default_project)
if project.module_path is not None:
module_paths.add(project.module_path)
source_projects.add(project)

def project_uses_client_parent(project: Project, projects: Dict[str, Project]) -> bool:
while project.parent_pom is not None:
if project.parent_pom == 'com.azure:azure-client-sdk-parent':
return True
project = projects.get(project.parent_pom, default_project)

return False

def main():
parser = argparse.ArgumentParser(description='Generated an aggregate POM for a From Source run.')
parser.add_argument('--project-list', '--pl', type=str)
parser.add_argument('--set-pipeline-variable', type=str)
parser.add_argument('--set-skip-linting-projects', type=str)
args = parser.parse_args()
if args.project_list == None:
raise ValueError('Missing project list.')
start_time = time.time()
create_from_source_pom(args.project_list, args.set_pipeline_variable)
create_from_source_pom(args.project_list, args.set_pipeline_variable, args.set_skip_linting_projects)
elapsed_time = time.time() - start_time

print('Effective From Source POM File')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -497,8 +497,7 @@ Mono<Response<SendChatMessageResult>> sendMessage(SendChatMessageOptions options
return this.chatThreadClient.sendChatMessageWithResponseAsync(chatThreadId, options, context)
.onErrorMap(CommunicationErrorResponseException.class, e -> translateException(e))
.map(result -> new SimpleResponse<SendChatMessageResult>(result, (result.getValue())));
}
catch (RuntimeException ex) {
} catch (RuntimeException ex) {
return monoError(logger, ex);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ protected void beforeTest() {
}

@Test
public void getLedgerEntries()
{
public void getLedgerEntries() {
DynamicResponse response = client.getLedgerEntries().send();

assertEquals(200, response.getStatusCode());
Expand Down
2 changes: 2 additions & 0 deletions sdk/cosmos/azure-cosmos-benchmark/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ Licensed under the MIT License.
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<jacoco.min.linecoverage>0.01</jacoco.min.linecoverage>
<jacoco.min.branchcoverage>0.01</jacoco.min.branchcoverage>
<checkstyle.skip>true</checkstyle.skip>
<spotbugs.skip>true</spotbugs.skip>
<jacoco.skip>true</jacoco.skip>
<codesnippet.skip>false</codesnippet.skip>
<javadocDoclet></javadocDoclet>
Expand Down
1 change: 1 addition & 0 deletions sdk/cosmos/azure-cosmos-dotnet-benchmark/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ Licensed under the MIT License.
<jacoco.min.linecoverage>0.01</jacoco.min.linecoverage>
<jacoco.min.branchcoverage>0.01</jacoco.min.branchcoverage>
<checkstyle.skip>true</checkstyle.skip>
<spotbugs.skip>true</spotbugs.skip>
<jacoco.skip>true</jacoco.skip>
<codesnippet.skip>false</codesnippet.skip>
<javadocDoclet></javadocDoclet>
Expand Down
6 changes: 6 additions & 0 deletions sdk/e2e/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@
<relativePath>../parents/azure-client-sdk-parent</relativePath>
</parent>

<properties>
<relative.path.to.eng.folder>../..</relative.path.to.eng.folder>
<checkstyle.skip>true</checkstyle.skip>
<spotbugs.skip>true</spotbugs.skip>
</properties>

<dependencies>
<dependency>
<groupId>com.azure</groupId>
Expand Down
1 change: 1 addition & 0 deletions sdk/keyvault/azure-security-test-keyvault-jca/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

<properties>
<codesnippet.skip>false</codesnippet.skip>
<spotbugs.skip>true</spotbugs.skip>
<javadocDoclet></javadocDoclet>
<javadocDocletOptions></javadocDocletOptions>
</properties>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

import com.azure.security.keyvault.jca.KeyVaultJcaProvider;

/**
* Test modularization.
*/
public class TestModularization {
/**
* Simply test compiling ok.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -482,20 +482,15 @@ public T getValue() {
private static <T, Y> T fromGeneratedGeneric(Y value) {
if (value == null) {
return null;
}
else if (value instanceof com.azure.mixedreality.remoterendering.implementation.models.Conversion) {
} else if (value instanceof com.azure.mixedreality.remoterendering.implementation.models.Conversion) {
return (T) fromGenerated((com.azure.mixedreality.remoterendering.implementation.models.Conversion) value);
}
else if (value instanceof SessionProperties) {
} else if (value instanceof SessionProperties) {
return (T) fromGenerated((SessionProperties) value);
}
else if (value instanceof com.azure.mixedreality.remoterendering.implementation.models.Error) {
} else if (value instanceof com.azure.mixedreality.remoterendering.implementation.models.Error) {
return (T) fromGenerated((com.azure.mixedreality.remoterendering.implementation.models.Error) value);
}
else if (value instanceof com.azure.mixedreality.remoterendering.implementation.models.ConversionSettings) {
} else if (value instanceof com.azure.mixedreality.remoterendering.implementation.models.ConversionSettings) {
return (T) fromGenerated((com.azure.mixedreality.remoterendering.implementation.models.ConversionSettings) value);
}
else {
} else {
// throw?
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ public RemoteRenderingAsyncClient buildAsyncClient() {
if (accessToken == null) {
MixedRealityStsAsyncClient stsClient = stsBuilder.buildAsyncClient();
builder.addPolicy(new BearerTokenAuthenticationPolicy(r -> stsClient.getToken(), scope));
}
else {
} else {
builder.addPolicy(new BearerTokenAuthenticationPolicy(r -> Mono.just(this.accessToken), scope));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ String getServiceEndpoint() {
String getRandomId(String playback) {
if (!interceptorManager.isPlaybackMode()) {
return UUID.randomUUID().toString();
}
else {
} else {
return playback;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,8 +444,7 @@ interface Update<ParentT>
UpdateStages.WithAutoScaling<ParentT>,
UpdateStages.WithAgentPoolMode<ParentT>,
UpdateStages.WithDiskType<ParentT>,
UpdateStages.WithTags<ParentT>
{
UpdateStages.WithTags<ParentT> {
}

/** Grouping of agent pool update stages. */
Expand Down

0 comments on commit 79ba9a0

Please sign in to comment.