Merge pull request #162 from uqbar-project/test-parameters-err-handling

Mejora comportamiento comando test con parametros -f -d -t

src/commands/test.ts

@@ -1,4 +1,4 @@
-import { bold } from 'chalk'
+import { bold, red } from 'chalk'
 import { time, timeEnd } from 'console'
 import logger from 'loglevel'
 import { Entity, Environment, Node, Test, is, match, when, WRENatives as natives, interpret, Describe } from 'wollok-ts'
@@ -17,32 +17,54 @@ export type Options = {
   skipValidations: boolean
 }
 
+class TestSearchMissError extends Error{}
+
 export function validateParameters(filter: string | undefined, { file, describe, test }: Options): void {
   if (filter && (file || describe || test)) throw new Error('You should either use filter by full name or file/describe/test.')
 }
 
+export function matchingTestDescription(filter: string | undefined, options: Options): string {
+  if(filter) return `matching ${valueDescription(filter)}`
+  if(options.file || options.describe || options.test) {
+    const stringifiedOrWildcard = (value?: string) => value ? `'${value}'` : '*'
+    return `matching ${valueDescription([options.file, options.describe, options.test].map(stringifiedOrWildcard).join('.'))}`
+  }
+  return ''
+}
+
 export function sanitize(value?: string): string | undefined {
   return value?.replaceAll('"', '')
 }
 
 export function getTarget(environment: Environment, filter: string | undefined, options: Options): Test[] {
-  const possibleTargets = getBaseNode(environment, filter, options).descendants.filter(getTestFilter(filter, options))
-  const onlyTarget = possibleTargets.find((test: Test) => test.isOnly)
-  const testMatches = (filter: string) => (test: Test) => !filter || sanitize(test.fullyQualifiedName)!.includes(filter)
-  const filterTest = sanitize(filter) ?? ''
-  return onlyTarget ? [onlyTarget] : possibleTargets.filter(testMatches(filterTest))
+  let possibleTargets: Test[]
+  try {
+    possibleTargets = getBaseNode(environment, filter, options).descendants.filter(getTestFilter(filter, options))
+    const onlyTarget = possibleTargets.find((test: Test) => test.isOnly)
+    const testMatches = (filter: string) => (test: Test) => !filter || sanitize(test.fullyQualifiedName)!.includes(filter)
+    const filterTest = sanitize(filter) ?? ''
+    return onlyTarget ? [onlyTarget] : possibleTargets.filter(testMatches(filterTest))
+  } catch(e: any){
+    if(e instanceof TestSearchMissError){
+      logger.error(red(bold(e.message)))
+      return []
+    }
+    throw e
+  }
 }
 
 function getBaseNode(environment: Environment, filter: string | undefined, options: Options): Environment | Package | Describe {
   if (filter) return environment
 
   const { file, describe } = options
-  let nodeToFilter: Environment | Package | Describe = environment
+  let nodeToFilter: Environment | Package | Describe | undefined = environment
   if (file) {
-    nodeToFilter = nodeToFilter.descendants.find(node => node.is(Package) && node.name === file) as Package | undefined ?? nodeToFilter
+    nodeToFilter = nodeToFilter.descendants.find(node => node.is(Package) && node.fileName === file) as Package | undefined
+    if(!nodeToFilter) throw new TestSearchMissError(`File '${file}' not found`)
   }
   if (describe) {
-    nodeToFilter = nodeToFilter.descendants.find(node => node.is(Describe) && node.name === `"${describe}"`) as Describe | undefined ?? nodeToFilter
+    nodeToFilter = nodeToFilter.descendants.find(node => node.is(Describe) && node.name === `"${describe}"`) as Describe | undefined
+    if(!nodeToFilter) throw new TestSearchMissError(`Describe '${describe}' not found`)
   }
   return nodeToFilter
 }
@@ -62,7 +84,9 @@ export default async function (filter: string | undefined, options: Options): Pr
 
     const timeMeasurer = new TimeMeasurer()
     const { project, skipValidations } = options
-    const runAllTestsDescription = `${testIcon} Running all tests ${filter ? `matching ${valueDescription(filter)} ` : ''}on ${valueDescription(project)}`
+
+    const matchLog = matchingTestDescription(filter, options)
+    const runAllTestsDescription = `${testIcon} Running all tests${matchLog ? ` ${matchLog} `: ' '}on ${valueDescription(project)}`
 
     logger.info(runAllTestsDescription)
 

src/index.ts

@@ -33,7 +33,7 @@ program.command('test')
   .description('Run Wollok tests')
   .argument('[filter]', 'filter pattern for a test, describe or package')
   .option('-p, --project [project]', 'path to project', process.cwd())
-  .option('-f, --file [file]', 'path to file', '')
+  .option('-f, --file [file]', 'path to file relative to the project', '')
   .option('-d, --describe [describe]', 'describe to run', '')
   .option('-t, --test [test]', 'test to run', '')
   .option('--skipValidations', 'skip code validation', false)

test/test.test.ts

@@ -1,11 +1,11 @@
 import { expect } from 'chai'
+import logger from 'loglevel'
 import { join } from 'path'
-import { buildEnvironmentForProject } from '../src/utils'
-import test, { getTarget, sanitize, tabulationForNode, validateParameters } from '../src/commands/test'
+import sinon from 'sinon'
 import { Environment } from 'wollok-ts'
-import logger from 'loglevel'
+import test, { getTarget, matchingTestDescription, sanitize, tabulationForNode, validateParameters } from '../src/commands/test'
 import { logger as fileLogger } from '../src/logger'
-import sinon from 'sinon'
+import { buildEnvironmentForProject } from '../src/utils'
 import { spyCalledWithSubstring } from './assertions'
 
 describe('Test', () => {
@@ -121,7 +121,7 @@ describe('Test', () => {
         it('should filter by file using file option', () => {
           const tests = getTarget(environment, undefined, {
             ...emptyOptions,
-            file: 'test-one',
+            file: 'test-one.wtest',
           })
           expect(tests.length).to.equal(3)
           expect(tests[0].name).to.equal('"a test"')
@@ -132,7 +132,7 @@ describe('Test', () => {
         it('should filter by file & describe using file & describe option', () => {
           const tests = getTarget(environment, undefined, {
             ...emptyOptions,
-            file: 'test-one',
+            file: 'test-one.wtest',
             describe: 'this describe',
           })
           expect(tests.length).to.equal(3)
@@ -144,7 +144,7 @@ describe('Test', () => {
         it('should filter by file & describe & test using file & describe & test option', () => {
           const tests = getTarget(environment, undefined, {
             ...emptyOptions,
-            file: 'test-one',
+            file: 'test-one.wtest',
             describe: 'this describe',
             test: 'another test',
           })
@@ -195,7 +195,7 @@ describe('Test', () => {
       it('should execute single test when running a file using file option', () => {
         const tests = getTarget(environment, undefined, {
           ...emptyOptions,
-          file: 'only-file',
+          file: 'only-file.wtest',
         })
         expect(tests.length).to.equal(1)
         expect(tests[0].name).to.equal('"this is the one"')
@@ -250,6 +250,43 @@ describe('Test', () => {
 
   })
 
+  describe('matching test description', () => {
+    const emptyOptions = {
+      project: '',
+      skipValidations: false,
+      file: undefined,
+      describe: undefined,
+      test: undefined,
+    }
+
+
+    it('should return empty string if no filter or options are passed', () => {
+      expect(matchingTestDescription(undefined, emptyOptions)).to.equal('')
+    })
+
+    it('should return filter description if filter is passed', () => {
+      expect(matchingTestDescription('some test', emptyOptions)).to.include('some test')
+    })
+
+    it('should return options descriptions if options are passed', () => {
+      expect(matchingTestDescription(undefined, {
+        ...emptyOptions,
+        file: 'test-one.wtest',
+        describe: 'this describe',
+        test: 'another test',
+      })).to.include('\'test-one.wtest\'.\'this describe\'.\'another test\'')
+    })
+
+    it('should return options descriptions with wildcards if options are missing', () => {
+      expect(matchingTestDescription(undefined, {
+        ...emptyOptions,
+        file: undefined,
+        describe: 'this discribe',
+        test: undefined,
+      })).to.include('*.\'this discribe\'.*')
+    })
+  })
+
   describe('tabulations for node', () => {
 
     it('should work for root package', () => {
@@ -265,6 +302,7 @@ describe('Test', () => {
 
     let fileLoggerInfoSpy: sinon.SinonStub
     let loggerInfoSpy: sinon.SinonStub
+    let loggerErrorSpy: sinon.SinonStub
     let processExitSpy: sinon.SinonStub
 
     const projectPath = join('examples', 'test-examples', 'normal-case')
@@ -281,6 +319,7 @@ describe('Test', () => {
       loggerInfoSpy = sinon.stub(logger, 'info')
       fileLoggerInfoSpy = sinon.stub(fileLogger, 'info')
       processExitSpy = sinon.stub(process, 'exit')
+      loggerErrorSpy = sinon.stub(logger, 'error')
     })
 
     afterEach(() => {
@@ -290,7 +329,7 @@ describe('Test', () => {
     it('passes all the tests successfully and exits normally', async () => {
       await test(undefined, {
         ...emptyOptions,
-        file: 'test-one',
+        file: 'test-one.wtest',
       })
 
       expect(processExitSpy.callCount).to.equal(0)
@@ -301,6 +340,29 @@ describe('Test', () => {
       expect(fileLoggerInfoSpy.firstCall.firstArg.result).to.deep.equal({ ok: 3, failed: 0 })
     })
 
+    it('passing a wrong filename runs no tests and logs a warning', async () => {
+      await test(undefined, {
+        ...emptyOptions,
+        file: 'non-existing-file.wtest',
+      })
+
+      expect(processExitSpy.callCount).to.equal(0)
+      expect(spyCalledWithSubstring(loggerInfoSpy, 'Running 0 tests')).to.be.true
+      expect(spyCalledWithSubstring(loggerErrorSpy, 'File \'non-existing-file.wtest\' not found')).to.be.true
+    })
+
+    it('passing a wrong describe runs no tests and logs a warning', async () => {
+      await test(undefined, {
+        ...emptyOptions,
+        file: 'test-one.wtest',
+        describe: 'non-existing-describe',
+      })
+
+      expect(processExitSpy.callCount).to.equal(0)
+      expect(spyCalledWithSubstring(loggerInfoSpy, 'Running 0 tests')).to.be.true
+      expect(spyCalledWithSubstring(loggerErrorSpy, 'Describe \'non-existing-describe\' not found')).to.be.true
+    })
+
     it('returns exit code 2 if one or more tests fail', async () => {
       await test(undefined, emptyOptions)