From 036733a38381fd6529c5af7774fc472558b37dbd Mon Sep 17 00:00:00 2001 From: Mark Johnson <739719+virgofx@users.noreply.github.com> Date: Mon, 28 Oct 2024 15:37:17 -0700 Subject: [PATCH] feat(tests): add tests for file-util.ts (#100) --- .github/workflows/test.yml | 2 +- __tests__/config.test.ts | 2 +- __tests__/file-util.test.ts | 318 ++++++++++++++++++++++++++++++++++++ assets/coverage-badge.svg | 2 +- sonar-project.properties | 2 +- src/file-util.ts | 45 +++-- src/releases.ts | 8 +- 7 files changed, 358 insertions(+), 21 deletions(-) create mode 100644 __tests__/file-util.test.ts diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 84309bf..4f55331 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3,7 +3,7 @@ on: # a pull request. push: branches: - - master + - main pull_request: types: [opened, synchronize, reopened] diff --git a/__tests__/config.test.ts b/__tests__/config.test.ts index d6b015b..2304bed 100644 --- a/__tests__/config.test.ts +++ b/__tests__/config.test.ts @@ -8,7 +8,7 @@ type InputMap = { [key: string]: string; }; -describe('config', () => { +describe('config.ts', () => { const defaultInputs: InputMap = { 'major-keywords': 'BREAKING CHANGE,!', 'minor-keywords': 'feat,feature', diff --git a/__tests__/file-util.test.ts b/__tests__/file-util.test.ts new file mode 100644 index 0000000..c41e287 --- /dev/null +++ b/__tests__/file-util.test.ts @@ -0,0 +1,318 @@ +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { copyModuleContents, removeDirectoryContents, shouldExcludeFile } from '../src/file-util'; + +describe('file-util.ts', () => { + let tempDir: string; + + beforeEach(() => { + // Create a temporary directory before each test + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'test-dir-')); + }); + + afterEach(() => { + // Remove temporary directory + fs.rmSync(tempDir, { recursive: true }); + }); + + describe('shouldExcludeFile', () => { + it('should exclude file when pattern matches', () => { + const baseDirectory = tempDir; + const filePath = path.join(tempDir, 'file.txt'); + const excludePatterns = ['*.txt']; + + expect(shouldExcludeFile(baseDirectory, filePath, excludePatterns)).toBe(true); + }); + + it('should not exclude file when pattern does not match', () => { + const baseDirectory = tempDir; + const filePath = path.join(tempDir, 'file.txt'); + const excludePatterns = ['*.js']; + + expect(shouldExcludeFile(baseDirectory, filePath, excludePatterns)).toBe(false); + }); + + it('should handle relative paths correctly', () => { + const baseDirectory = tempDir; + const filePath = path.join(tempDir, 'subdir', 'file.txt'); + const excludePatterns = ['subdir/*.txt']; + + expect(shouldExcludeFile(baseDirectory, filePath, excludePatterns)).toBe(true); + }); + + it('should handle exclusion pattern: *.md', () => { + const baseDirectory = tempDir; + const filePath1 = path.join(tempDir, 'README.md'); + const filePath2 = path.join(tempDir, 'nested', 'README.md'); + const excludePatterns = ['*.md']; + + expect(shouldExcludeFile(baseDirectory, filePath1, excludePatterns)).toBe(true); + expect(shouldExcludeFile(baseDirectory, filePath2, excludePatterns)).toBe(true); + }); + + it('should handle exclusion pattern: **/*.md', () => { + const baseDirectory = tempDir; + const filePath1 = path.join(tempDir, 'README.md'); + const filePath2 = path.join(tempDir, 'nested', 'README.md'); + const excludePatterns = ['**/*.md']; + + expect(shouldExcludeFile(baseDirectory, filePath1, excludePatterns)).toBe(true); + expect(shouldExcludeFile(baseDirectory, filePath2, excludePatterns)).toBe(true); + }); + + it('should handle exclusion pattern: tests/**', () => { + const baseDirectory = tempDir; + const filePath1 = path.join(tempDir, 'tests/config.test.ts'); + const filePath2 = path.join(tempDir, 'tests2/config.test.ts'); + const filePath3 = path.join(tempDir, 'tests2/tests/config.test.ts'); + const excludePatterns = ['tests/**']; + + expect(shouldExcludeFile(baseDirectory, filePath1, excludePatterns)).toBe(true); + expect(shouldExcludeFile(baseDirectory, filePath2, excludePatterns)).toBe(false); + expect(shouldExcludeFile(baseDirectory, filePath3, excludePatterns)).toBe(false); + }); + + it('should handle exclusion pattern: **/tests/**', () => { + const baseDirectory = tempDir; + const filePath1 = path.join(tempDir, 'tests/config.test.ts'); + const filePath2 = path.join(tempDir, 'tests2/config.test.ts'); + const filePath3 = path.join(tempDir, 'tests2/tests/config.test.ts'); + const excludePatterns = ['**/tests/**']; + + expect(shouldExcludeFile(baseDirectory, filePath1, excludePatterns)).toBe(true); + expect(shouldExcludeFile(baseDirectory, filePath2, excludePatterns)).toBe(false); + expect(shouldExcludeFile(baseDirectory, filePath3, excludePatterns)).toBe(true); + }); + }); + + describe('copyModuleContents', () => { + beforeEach(() => { + // Create src and dest directories for every test in this suite + fs.mkdirSync(path.join(tempDir, 'src'), { recursive: true }); + fs.mkdirSync(path.join(tempDir, 'dest'), { recursive: true }); + }); + + it('should copy directory contents excluding files that match patterns', () => { + const srcDirectory = path.join(tempDir, 'src'); + const destDirectory = path.join(tempDir, 'dest'); + const excludePatterns = ['*.txt']; + + // Create files in src directory + fs.writeFileSync(path.join(srcDirectory, 'file.txt'), 'Hello World!'); + fs.writeFileSync(path.join(srcDirectory, 'file.js'), 'console.log("Hello World!");'); + + // Now perform the copy operation + copyModuleContents(srcDirectory, destDirectory, excludePatterns); + + // Check that the file was copied + expect(fs.existsSync(path.join(destDirectory, 'file.txt'))).toBe(false); + expect(fs.existsSync(path.join(destDirectory, 'file.js'))).toBe(true); + }); + + it('should handle recursive directory copying', () => { + const srcDirectory = path.join(tempDir, 'src'); + const destDirectory = path.join(tempDir, 'dest'); + const excludePatterns: string[] = []; + + // Create source structure + fs.mkdirSync(path.join(srcDirectory, 'subdir'), { recursive: true }); + fs.writeFileSync(path.join(srcDirectory, 'file.txt'), 'Hello World!'); + fs.writeFileSync(path.join(srcDirectory, 'subdir', 'file.js'), 'console.log("Hello World!");'); + + // Perform the copy operation + copyModuleContents(srcDirectory, destDirectory, excludePatterns); + + // Validate the destination contents + expect(fs.existsSync(path.join(destDirectory, 'file.txt'))).toBe(true); + expect(fs.existsSync(path.join(destDirectory, 'subdir', 'file.js'))).toBe(true); + }); + + it('should copy files excluding multiple patterns', () => { + const srcDirectory = path.join(tempDir, 'src'); + const destDirectory = path.join(tempDir, 'dest'); + const excludePatterns = ['*.txt', '*.js']; + + fs.writeFileSync(path.join(srcDirectory, 'file.txt'), 'Hello World!'); + fs.writeFileSync(path.join(srcDirectory, 'file.js'), 'console.log("Hello World!");'); + fs.writeFileSync(path.join(srcDirectory, 'file.md'), 'This is a markdown file.'); + + copyModuleContents(srcDirectory, destDirectory, excludePatterns); + + expect(fs.existsSync(path.join(destDirectory, 'file.txt'))).toBe(false); + expect(fs.existsSync(path.join(destDirectory, 'file.js'))).toBe(false); + expect(fs.existsSync(path.join(destDirectory, 'file.md'))).toBe(true); + }); + + it('should handle copying from an empty directory', () => { + const srcDirectory = path.join(tempDir, 'src'); + const destDirectory = path.join(tempDir, 'dest'); + const excludePatterns = ['*.txt']; + + copyModuleContents(srcDirectory, destDirectory, excludePatterns); + + // Validate that the destination directory is still empty + expect(fs.readdirSync(destDirectory).length).toBe(0); + }); + + it('should throw an error if the source directory does not exist', () => { + const nonExistentSrcDirectory = path.join(tempDir, 'non-existent-src'); + const destDirectory = path.join(tempDir, 'dest'); + const excludePatterns = ['*.txt']; + + expect(() => { + copyModuleContents(nonExistentSrcDirectory, destDirectory, excludePatterns); + }).toThrow(); // Assuming your implementation throws an error for non-existent directories + }); + + it('should copy files that do not match any exclusion patterns', () => { + const srcDirectory = path.join(tempDir, 'src'); + const destDirectory = path.join(tempDir, 'dest'); + const excludePatterns = ['*.js']; + + fs.writeFileSync(path.join(srcDirectory, 'file.txt'), 'Hello World!'); + fs.writeFileSync(path.join(srcDirectory, 'file.js'), 'console.log("Hello World!");'); + + copyModuleContents(srcDirectory, destDirectory, excludePatterns); + + expect(fs.existsSync(path.join(destDirectory, 'file.txt'))).toBe(true); + expect(fs.existsSync(path.join(destDirectory, 'file.js'))).toBe(false); + }); + + it('should overwrite files in the destination if they have the same name and do not match exclusion patterns', () => { + const srcDirectory = path.join(tempDir, 'src'); + const destDirectory = path.join(tempDir, 'dest'); + const excludePatterns: string[] = []; + + fs.writeFileSync(path.join(srcDirectory, 'file.txt'), 'Hello World from source!'); + fs.writeFileSync(path.join(destDirectory, 'file.txt'), 'Hello World from destination!'); + + copyModuleContents(srcDirectory, destDirectory, excludePatterns); + + const destContent = fs.readFileSync(path.join(destDirectory, 'file.txt'), 'utf-8'); + expect(destContent).toBe('Hello World from source!'); + }); + }); + + describe('removeDirectoryContents', () => { + it('should remove directory contents except for specified exceptions', () => { + const directory = path.join(tempDir, 'dir'); + const exceptions = ['file.txt']; + + fs.mkdirSync(directory); + fs.writeFileSync(path.join(directory, 'file.txt'), 'Hello World!'); + fs.writeFileSync(path.join(directory, 'file.js'), 'console.log("Hello World!");'); + + removeDirectoryContents(directory, exceptions); + + expect(fs.existsSync(path.join(directory, 'file.txt'))).toBe(true); + expect(fs.existsSync(path.join(directory, 'file.js'))).toBe(false); + }); + + it('should handle recursive directory removal', () => { + const directory = path.join(tempDir, 'dir'); + const exceptions: string[] = []; + + fs.mkdirSync(directory); + fs.mkdirSync(path.join(directory, 'subdir')); + fs.writeFileSync(path.join(directory, 'file.txt'), 'Hello World!'); + fs.writeFileSync(path.join(directory, 'subdir', 'file.js'), 'console.log("Hello World!");'); + + removeDirectoryContents(directory, exceptions); + + expect(fs.existsSync(path.join(directory, 'file.txt'))).toBe(false); + expect(fs.existsSync(path.join(directory, 'subdir', 'file.js'))).toBe(false); + }); + + it('should handle exceptions correctly', () => { + const directory = path.join(tempDir, 'dir'); + const exceptions = ['file.txt', 'subdir']; + + fs.mkdirSync(directory); + fs.mkdirSync(path.join(directory, 'subdir')); + fs.writeFileSync(path.join(directory, 'file.txt'), 'Hello World!'); + fs.writeFileSync(path.join(directory, 'file.js'), 'console.log("Hello World!");'); + fs.writeFileSync(path.join(directory, 'subdir', 'file.js'), 'console.log("Hello World!");'); + + removeDirectoryContents(directory, exceptions); + + expect(fs.existsSync(path.join(directory, 'file.txt'))).toBe(true); + expect(fs.existsSync(path.join(directory, 'file.js'))).toBe(false); + expect(fs.existsSync(path.join(directory, 'subdir', 'file.js'))).toBe(true); + }); + + it('should handle an empty directory', () => { + const directory = path.join(tempDir, 'dir'); + const exceptions: string[] = []; + + fs.mkdirSync(directory); // Create an empty directory + removeDirectoryContents(directory, exceptions); + + // Validate that the directory is still empty + expect(fs.readdirSync(directory).length).toBe(0); + }); + + it('should not remove if only exceptions are present', () => { + const directory = path.join(tempDir, 'dir'); + const exceptions = ['file.txt']; + + fs.mkdirSync(directory); + fs.writeFileSync(path.join(directory, 'file.txt'), 'Hello World!'); + + removeDirectoryContents(directory, exceptions); + + expect(fs.existsSync(path.join(directory, 'file.txt'))).toBe(true); + expect(fs.readdirSync(directory).length).toBe(1); // Only the exception should exist + }); + + it('should handle nested exceptions correctly', () => { + const directory = path.join(tempDir, 'dir'); + const exceptions = ['subdir']; + + fs.mkdirSync(directory); + fs.mkdirSync(path.join(directory, 'subdir')); + fs.writeFileSync(path.join(directory, 'file.txt'), 'Hello World!'); + fs.writeFileSync(path.join(directory, 'subdir', 'file.js'), 'console.log("Hello World!");'); + + removeDirectoryContents(directory, exceptions); + + expect(fs.existsSync(path.join(directory, 'subdir'))).toBe(true); + expect(fs.existsSync(path.join(directory, 'file.txt'))).toBe(false); + expect(fs.existsSync(path.join(directory, 'subdir', 'file.js'))).toBe(true); + }); + + it('should not throw an error if the directory does not exist', () => { + const nonExistentDirectory = path.join(tempDir, 'non-existent-dir'); + const exceptions = ['file.txt']; + + expect(() => { + removeDirectoryContents(nonExistentDirectory, exceptions); + }).not.toThrow(); // Ensure no error is thrown + }); + + it('should handle exceptions that do not exist in the directory', () => { + const directory = path.join(tempDir, 'dir'); + const exceptions = ['file.txt']; + + fs.mkdirSync(directory); + fs.writeFileSync(path.join(directory, 'file.js'), 'console.log("Hello World!");'); + + removeDirectoryContents(directory, exceptions); + + expect(fs.existsSync(path.join(directory, 'file.js'))).toBe(false); + }); + + it('should remove directory contents when no exceptions specified', () => { + const directory = path.join(tempDir, 'dir'); + + fs.mkdirSync(directory); + fs.writeFileSync(path.join(directory, 'file.txt'), 'Hello World!'); + fs.writeFileSync(path.join(directory, 'file.js'), 'console.log("Hello World!");'); + + removeDirectoryContents(directory); + + expect(fs.existsSync(path.join(directory, 'file.txt'))).toBe(false); + expect(fs.existsSync(path.join(directory, 'file.js'))).toBe(false); + }); + }); +}); diff --git a/assets/coverage-badge.svg b/assets/coverage-badge.svg index c5e6ef5..7d4e6ce 100644 --- a/assets/coverage-badge.svg +++ b/assets/coverage-badge.svg @@ -1 +1 @@ -Coverage: 4.98%Coverage4.98% \ No newline at end of file +Coverage: 9.16%Coverage9.16% \ No newline at end of file diff --git a/sonar-project.properties b/sonar-project.properties index 7167e8e..f22903a 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -1,6 +1,6 @@ sonar.organization=techpivot sonar.projectKey=terraform-module-releaser -sonar.projectName=Terraform Mmodule Releaser +sonar.projectName=Terraform Module Releaser sonar.sourceEncoding=UTF-8 diff --git a/src/file-util.ts b/src/file-util.ts index 578d02b..8c56c4c 100644 --- a/src/file-util.ts +++ b/src/file-util.ts @@ -2,7 +2,6 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { info } from '@actions/core'; import { minimatch } from 'minimatch'; -import { config } from './config'; /** * Checks if a file should be excluded from matching based on the defined exclude patterns @@ -15,7 +14,18 @@ import { config } from './config'; */ export function shouldExcludeFile(baseDirectory: string, filePath: string, excludePatterns: string[]): boolean { const relativePath = path.relative(baseDirectory, filePath); - return excludePatterns.some((pattern: string) => minimatch(relativePath, pattern, { matchBase: true })); + + // Expand patterns to include both directories and their contents, then remove duplicates + const expandedPatterns = Array.from( + new Set( + excludePatterns.flatMap((pattern) => [ + pattern, // Original pattern + pattern.replace(/\/(?:\*\*)?$/, ''), // Match directories themselves, like `tests2/` + ]), + ), + ); + + return expandedPatterns.some((pattern: string) => minimatch(relativePath, pattern, { matchBase: true })); } /** @@ -24,10 +34,16 @@ export function shouldExcludeFile(baseDirectory: string, filePath: string, exclu * * @param {string} directory - The directory to copy from. * @param {string} tmpDir - The temporary directory to copy to. + * @param {string[]} excludePatterns - An array of patterns to match against for exclusion. * @param {string} [baseDirectory] - The base directory for exclusion pattern matching. * Defaults to the source directory if not provided. */ -export function copyModuleContents(directory: string, tmpDir: string, baseDirectory?: string) { +export function copyModuleContents( + directory: string, + tmpDir: string, + excludePatterns: string[], + baseDirectory?: string, +) { const baseDir = baseDirectory ?? directory; // Read the directory contents @@ -43,8 +59,8 @@ export function copyModuleContents(directory: string, tmpDir: string, baseDirect const newDir = path.join(tmpDir, file); fs.mkdirSync(newDir, { recursive: true }); // Note: Important we pass the original base directory. - copyModuleContents(filePath, newDir, baseDir); // Recursion for directory contents - } else if (!shouldExcludeFile(baseDir, filePath, config.moduleAssetExcludePatterns)) { + copyModuleContents(filePath, newDir, excludePatterns, baseDir); // Recursion for directory contents + } else if (!shouldExcludeFile(baseDir, filePath, excludePatterns)) { // Handle file copying fs.copyFileSync(filePath, path.join(tmpDir, file)); } else { @@ -90,15 +106,18 @@ export function copyModuleContents(directory: string, tmpDir: string, baseDirect * // and the `important-file.txt` file. */ export function removeDirectoryContents(directory: string, exceptions: string[] = []): void { - if (fs.existsSync(directory)) { - for (const item of fs.readdirSync(directory)) { - const itemPath = path.join(directory, item); + if (!fs.existsSync(directory)) { + return; + } + + for (const item of fs.readdirSync(directory)) { + const itemPath = path.join(directory, item); - // Skip removal for items listed in the exceptions array - if (!exceptions.includes(item)) { - fs.rmSync(itemPath, { recursive: true, force: true }); - } + // Skip removal for items listed in the exceptions array + if (!shouldExcludeFile(directory, itemPath, exceptions)) { + //if (!exceptions.includes(item)) { + fs.rmSync(itemPath, { recursive: true, force: true }); } - info(`Removed contents of directory [${directory}], preserving items: ${exceptions.join(', ')}`); } + info(`Removed contents of directory [${directory}], preserving items: ${exceptions.join(', ')}`); } diff --git a/src/releases.ts b/src/releases.ts index cf47a26..7b1f651 100644 --- a/src/releases.ts +++ b/src/releases.ts @@ -56,8 +56,8 @@ export async function getAllReleases(): Promise { for (const release of response.data) { releases.push({ id: release.id, - title: release.name || '', // same as tag as defined in our pull request for now (no need for tag) - body: release.body || '', + title: release.name ?? '', // same as tag as defined in our pull request for now (no need for tag) + body: release.body ?? '', }); } } @@ -117,7 +117,7 @@ export async function createTaggedRelease( try { for (const module of terraformChangedModules) { const { moduleName, directory, releaseType, nextTag, nextTagVersion } = module; - const tmpDir = path.join(process.env.RUNNER_TEMP || '', 'tmp', moduleName); + const tmpDir = path.join(process.env.RUNNER_TEMP ?? '', 'tmp', moduleName); info(`Release type: ${releaseType}`); info(`Next tag version: ${nextTag}`); @@ -127,7 +127,7 @@ export async function createTaggedRelease( info(`Creating temp directory: ${tmpDir}`); // Copy the module's contents to the temporary directory, excluding specified patterns - copyModuleContents(directory, tmpDir); + copyModuleContents(directory, tmpDir, config.moduleAssetExcludePatterns); // Copy the module's .git directory fs.cpSync(path.join(workspaceDir, '.git'), path.join(tmpDir, '.git'), { recursive: true });