test: improve test suite (#20)

.github/workflows/test.yml

@@ -13,7 +13,7 @@ jobs:
 
     strategy:
       matrix:
-        node-version: [16.x, 18.x]
+        node-version: [18.x, 20.x]
 
     steps:
       - uses: actions/checkout@v3

.npmignore

@@ -1,5 +1,3 @@
 .github
 .husky
-
 test
-sample-data

src/processor.ts

@@ -30,7 +30,7 @@ export class Processor {
           if (messages.length) {
             // Add the warning messages to the list together with the natspec location
             warnings.push({
-              location: this.formatLocation(sourceUnit.absolutePath, fileContent, contract, node),
+              location: this.formatLocation(sourceUnit.absolutePath, fileContent, contract.name, node),
               messages,
             });
           }
@@ -54,15 +54,14 @@ export class Processor {
   }
 
   validateNatspec(node: NodeToProcess): string[] {
-    if (!node) return [];
     const nodeNatspec = parseNodeNatspec(node);
     return this.validator.validate(node, nodeNatspec);
   }
 
-  formatLocation(filePath: string, fileContent: string, contract: ContractDefinition, node: NodeToProcess): string {
+  formatLocation(filePath: string, fileContent: string, contractName: string, node: NodeToProcess): string {
     // the constructor function definition does not have a name, but it has kind: 'constructor'
     const nodeName = node instanceof FunctionDefinition ? node.name || node.kind : node.name;
     const line: number = getLineNumberFromSrc(fileContent, node.src);
-    return `${filePath}:${line}\n${contract.name}:${nodeName}`;
+    return `${filePath}:${line}\n${contractName}:${nodeName}`;
   }
 }

test/mocks.ts

@@ -1,5 +1,5 @@
 import { SourceUnit, ContractDefinition, FunctionDefinition } from 'solc-typed-ast';
-import { Natspec, NodeToProcess } from '../src/types';
+import { Natspec, NodeToProcess, Config } from '../src/types';
 
 export function mockNatspec(mockNatspec: Partial<Natspec>): Natspec {
   const natspec: Natspec = {
@@ -31,3 +31,7 @@ export function mockSourceUnit(mockSourceUnit: Partial<SourceUnit>): SourceUnit
 export function mockContractDefinition(mockContractDefinition: Partial<ContractDefinition>): ContractDefinition {
   return mockContractDefinition as ContractDefinition;
 }
+
+export function mockConfig(mockConfig: Partial<Config>): Config {
+  return mockConfig as Config;
+}

test/parser.test.ts

@@ -8,7 +8,7 @@ describe('Parser', () => {
 
   describe('Contract', () => {
     beforeAll(async () => {
-      const compileResult = await getFileCompiledSource('sample-data/ParserTest.sol');
+      const compileResult = await getFileCompiledSource('test/contracts/ParserTest.sol');
       contract = compileResult.vContracts.find(({ name }) => name === 'ParserTest')!;
     });
 
@@ -167,7 +167,7 @@ describe('Parser', () => {
 
   describe('Interface', () => {
     beforeAll(async () => {
-      const compileResult = await getFileCompiledSource('sample-data/ParserTest.sol');
+      const compileResult = await getFileCompiledSource('test/contracts/ParserTest.sol');
       contract = compileResult.vContracts.find(({ name }) => name === 'IParserTest')!;
     });
 
@@ -302,7 +302,7 @@ describe('Parser', () => {
 
   describe('Contract with invalid natspec', () => {
     beforeAll(async () => {
-      const compileResult = await getFileCompiledSource('sample-data/ParserTest.sol');
+      const compileResult = await getFileCompiledSource('test/contracts/ParserTest.sol');
       contract = compileResult.vContracts.find(({ name }) => name === 'ParserTestFunny')!;
     });
 

test/processor.test.ts

@@ -1,61 +1,20 @@
-import { ContractDefinition, FunctionDefinition, UserDefinedType, UsingForDirective } from 'solc-typed-ast';
-import { Config } from '../src/types';
-import { getFileCompiledSource } from './utils';
+import { faker } from '@faker-js/faker';
+import { ContractDefinition, FunctionDefinition, UserDefinedType, UsingForDirective, FunctionKind } from 'solc-typed-ast';
+import * as utils from '../src/utils';
 import { Processor } from '../src/processor';
 import { Validator } from '../src/validator';
+import { getFileCompiledSource } from './utils';
+import { mockFunctionDefinition, mockNodeToProcess, mockConfig, mockNatspec } from './mocks';
 
 describe('Processor', () => {
-  let processor: Processor;
-
-  beforeAll(() => {
-    const config: Config = {
-      root: '.',
-      include: './sample-data',
-      exclude: '',
-      enforceInheritdoc: false,
-      constructorNatspec: false,
-    };
-
-    const validator = new Validator(config);
-    processor = new Processor(validator);
-  });
-
-  // TODO: Fix these tests
-  // describe('formatLocation', () => {
-  //   const absolutePath = faker.system.filePath();
-  //   const contractName = faker.lorem.word();
-  //   const nodeName = faker.lorem.word();
-
-  //   const sourceUnit = mockSourceUnit({
-  //     absolutePath: absolutePath,
-  //   });
-
-  //   const contract = mockContractDefinition({
-  //     name: contractName,
-  //   });
-
-  //   it('should format the location of the node', () => {
-  //     const node = mockNodeToProcess({
-  //       name: nodeName,
-  //     });
-
-  //     expect(processor.formatLocation(node, sourceUnit, contract)).toEqual(`${absolutePath}\n${contractName}:${nodeName}`);
-  //   });
-
-  //   it('should format the location of a constructor', () => {
-  //     const node = mockFunctionDefinition({
-  //       kind: FunctionKind.Constructor,
-  //     });
-
-  //     expect(processor.formatLocation(node, sourceUnit, contract)).toEqual(`${absolutePath}\n${contractName}:constructor`);
-  //   });
-  // });
+  const validator = new Validator(mockConfig({}));
+  const processor = new Processor(validator);
 
   describe('selectEligibleNodes', () => {
     let contract: ContractDefinition;
 
     beforeAll(async () => {
-      const compileResult = await getFileCompiledSource('sample-data/ParserTest.sol');
+      const compileResult = await getFileCompiledSource('test/contracts/ParserTest.sol');
       contract = compileResult.vContracts.find(({ name }) => name === 'ParserTest')!;
     });
 
@@ -130,4 +89,61 @@ describe('Processor', () => {
       expect(eligibleNodes.filter((node) => node instanceof FunctionDefinition && node.isConstructor).length).toEqual(1);
     });
   });
+
+  describe('validateNatspec', () => {
+    const node = mockNodeToProcess({});
+    const nodeNatspec = mockNatspec({});
+
+    it('should parse the natspec of the node', () => {
+      const parseNodeNatspecSpy = jest.spyOn(utils, 'parseNodeNatspec').mockImplementation((_) => nodeNatspec);
+
+      processor.validateNatspec(node);
+
+      expect(parseNodeNatspecSpy).toHaveBeenCalledWith(node);
+    });
+
+    it('should validate the natspec of the node', () => {
+      const messages = [faker.lorem.sentence(), faker.lorem.sentence()];
+      const validateSpy = jest.spyOn(validator, 'validate').mockImplementation((_, __) => messages);
+
+      const validatedNatspec = processor.validateNatspec(node);
+
+      expect(validatedNatspec).toEqual(messages);
+      expect(validateSpy).toHaveBeenCalledWith(node, nodeNatspec);
+    });
+  });
+
+  describe('formatLocation', () => {
+    const absolutePath = faker.system.filePath();
+    const contractName = faker.lorem.word();
+    const nodeName = faker.lorem.word();
+    const fileContent = faker.lorem.sentence();
+    const lineNumber = faker.number.int(100);
+    const src = '${lineNumber}:1:0';
+    const getLineNumberFromSrcSpy = jest.spyOn(utils, 'getLineNumberFromSrc').mockImplementation(() => lineNumber);
+
+    it('should format the location of a node', () => {
+      const node = mockNodeToProcess({ name: nodeName, src: src });
+      const formattedLocation = processor.formatLocation(absolutePath, fileContent, contractName, node);
+
+      expect(getLineNumberFromSrcSpy).toHaveBeenCalledWith(fileContent, src);
+      expect(formattedLocation).toEqual(`${absolutePath}:${lineNumber}\n${contractName}:${nodeName}`);
+    });
+
+    it('should format the location of a constructor', () => {
+      const node = mockFunctionDefinition({ kind: FunctionKind.Constructor, src: src });
+      const formattedLocation = processor.formatLocation(absolutePath, fileContent, contractName, node);
+
+      expect(getLineNumberFromSrcSpy).toHaveBeenCalledWith(fileContent, src);
+      expect(formattedLocation).toEqual(`${absolutePath}:${lineNumber}\n${contractName}:constructor`);
+    });
+
+    it('should format the location of a function', () => {
+      const node = mockFunctionDefinition({ name: nodeName, src: src });
+      const formattedLocation = processor.formatLocation(absolutePath, fileContent, contractName, node);
+
+      expect(getLineNumberFromSrcSpy).toHaveBeenCalledWith(fileContent, src);
+      expect(formattedLocation).toEqual(`${absolutePath}:${lineNumber}\n${contractName}:${nodeName}`);
+    });
+  });
 });

test/utils.ts

@@ -4,3 +4,7 @@ export async function getFileCompiledSource(filePath: string): Promise<SourceUni
   const compiledFile = await compileSol(filePath, 'auto');
   return new ASTReader().read(compiledFile.data, ASTKind.Any, compiledFile.files)[0];
 }
+
+export function expectMissingTags(warnings: string[], tag: string, numberOfWarnings: number) {
+  expect(warnings.filter((warning) => warning.match(tag)).length).toBe(numberOfWarnings);
+}