Skip to content

Commit

Permalink
[Ownership] Print owner match also, not just owner (elastic#202704)
Browse files Browse the repository at this point in the history
## Summary

Resolves: elastic#202666

## For reviewers
To see the change, run this:
`node scripts/get_owners_for_file.js --file
test/functional/apps/console/_autocomplete.ts`
### Results:
#### Before
```
 succ elastic/kibana-management
```
#### After
```
 succ Found matching entry in .github/CODEOWNERS:
      test/functional/apps/console/*.ts elastic/kibana-management
```

---------

Co-authored-by: Robert Oskamp <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: David Olaru <[email protected]>
  • Loading branch information
4 people authored Dec 10, 2024
1 parent 2a76fe3 commit e706b66
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 17 deletions.
2 changes: 1 addition & 1 deletion packages/kbn-code-owners/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/

export type { PathWithOwners } from './src/file_code_owner';
export type { PathWithOwners, CodeOwnership } from './src/file_code_owner';
export {
getPathsWithOwnersReversed,
getCodeOwnersForFile,
Expand Down
15 changes: 9 additions & 6 deletions packages/kbn-code-owners/src/file_code_owner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export interface PathWithOwners {
teams: string;
ignorePattern: Ignore;
}
export type CodeOwnership = Partial<Pick<PathWithOwners, 'path' | 'teams'>> | undefined;

const existOrThrow = (targetFile: string) => {
if (existsSync(targetFile) === false)
throw createFailError(`Unable to determine code owners: file ${targetFile} Not Found`);
Expand Down Expand Up @@ -65,14 +67,13 @@ export function getPathsWithOwnersReversed(): PathWithOwners[] {
export function getCodeOwnersForFile(
filePath: string,
reversedCodeowners?: PathWithOwners[]
): string | undefined {
): CodeOwnership {
const pathsWithOwners = reversedCodeowners ?? getPathsWithOwnersReversed();

const match = pathsWithOwners.find((p) => p.ignorePattern.test(filePath).ignored);

return match?.teams;
if (match?.path && match.teams) return { path: match.path, teams: match.teams };
return;
}

const trimFrontSlash = (x: string): string => x.replace(/^\//, '');
/**
* Run the getCodeOwnersForFile() method above.
* Report back to the cli with either success and the owner(s), or a failure.
Expand All @@ -87,7 +88,9 @@ export async function runGetOwnersForFileCli() {
if (!targetFile) throw createFlagError(`Missing --file argument`);
existOrThrow(targetFile); // This call is duplicated in getPathsWithOwnersReversed(), so this is a short circuit
const result = getCodeOwnersForFile(targetFile);
if (result) log.success(result);
if (result)
log.success(`Found matching entry in .github/CODEOWNERS:
${trimFrontSlash(result?.path ? result.path : '')} ${result.teams}`);
else log.error(`Ownership of file [${targetFile}] is UNKNOWN`);
},
{
Expand Down
8 changes: 6 additions & 2 deletions packages/kbn-scout-reporting/src/reporting/playwright.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ import { ToolingLog } from '@kbn/tooling-log';
import { SCOUT_REPORT_OUTPUT_ROOT } from '@kbn/scout-info';
import stripANSI from 'strip-ansi';
import { REPO_ROOT } from '@kbn/repo-info';
import { PathWithOwners, getPathsWithOwnersReversed, getCodeOwnersForFile } from '@kbn/code-owners';
import {
type PathWithOwners,
getPathsWithOwnersReversed,
getCodeOwnersForFile,
} from '@kbn/code-owners';
import { generateTestRunId, getTestIDForTitle, ScoutReport, ScoutReportEventAction } from '.';
import { environmentMetadata } from '../datasources';

Expand Down Expand Up @@ -60,7 +64,7 @@ export class ScoutPlaywrightReporter implements Reporter {
}

private getFileOwners(filePath: string): string[] {
const concatenatedOwners = getCodeOwnersForFile(filePath, this.pathsWithOwners);
const concatenatedOwners = getCodeOwnersForFile(filePath, this.pathsWithOwners)?.teams;

if (concatenatedOwners === undefined) {
return [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ import {
ScoutReportEventAction,
datasources,
} from '@kbn/scout-reporting';
import { getCodeOwnersForFile, getPathsWithOwnersReversed, PathWithOwners } from '@kbn/code-owners';
import {
getCodeOwnersForFile,
getPathsWithOwnersReversed,
type PathWithOwners,
} from '@kbn/code-owners';
import { Runner, Test } from '../../../fake_mocha_types';

/**
Expand Down Expand Up @@ -64,7 +68,7 @@ export class ScoutFTRReporter {
}

private getFileOwners(filePath: string): string[] {
const concatenatedOwners = getCodeOwnersForFile(filePath, this.pathsWithOwners);
const concatenatedOwners = getCodeOwnersForFile(filePath, this.pathsWithOwners)?.teams;

if (concatenatedOwners === undefined) {
return [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@
import { run } from '@kbn/dev-cli-runner';
import { createFailError } from '@kbn/dev-cli-errors';
import { getRepoFiles } from '@kbn/get-repo-files';
import { getCodeOwnersForFile, getPathsWithOwnersReversed } from '@kbn/code-owners';
import {
getCodeOwnersForFile,
getPathsWithOwnersReversed,
type CodeOwnership,
} from '@kbn/code-owners';

const TEST_DIRECTORIES = ['test', 'x-pack/test', 'x-pack/test_serverless'];

Expand All @@ -36,10 +40,8 @@ export async function runCheckFtrCodeOwnersCli() {

const testFiles = await getRepoFiles(TEST_DIRECTORIES);
for (const { repoRel } of testFiles) {
const owners = getCodeOwnersForFile(repoRel, reversedCodeowners);
if (owners === undefined || owners === '') {
missingOwners.add(repoRel);
}
const owners: CodeOwnership = getCodeOwnersForFile(repoRel, reversedCodeowners);
if (owners === undefined || owners.teams === '') missingOwners.add(repoRel);
}

const timeSpent = fmtMs(performance.now() - start);
Expand Down
3 changes: 2 additions & 1 deletion packages/kbn-test/src/mocha/junit_report_generation.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,9 @@ export function setupJUnitReportGeneration(runner, options = {}) {
// adding code owners only for the failed test case
if (failed) {
const testCaseRelativePath = getPath(node);

const owners = getCodeOwnersForFile(testCaseRelativePath, reversedCodeowners);
attrs.owners = owners || ''; // empty string when no codeowners are defined
attrs.owners = owners?.teams || ''; // empty string when no codeowners are defined
}

return testsuitesEl.ele('testcase', attrs);
Expand Down

0 comments on commit e706b66

Please sign in to comment.