Merge pull request #215 from VisLab/update-tokenizer

Updating of the tests. @happy5214 I resolved the conflicts between the HED2 removal and this testing update PR.  I am going to go ahead and merge.  Note that the action to run the tests on the latest version of node took a really long time.  It seems that the  setup node GitHub action doesn't have the latest version of Node v23+ cached.  We will need to keep a eye on this.

tests/bidsTests.spec.js

@@ -11,11 +11,9 @@ import { bidsTestData } from './testData/bidsTests.data'
 import { shouldRun } from './testUtilities'
 
 // Ability to select individual tests to run
+const skipMap = new Map([['definition-tests', ['invalid-missing-definition-for-def', 'invalid-nested-definition']]])
 const runAll = true
-let onlyRun = new Map()
-if (!runAll) {
-  onlyRun = new Map([['invalid-tag-tests', ['invalid-bad-tag-in-JSON']]])
-}
+const runMap = new Map([['duplicate-tag-tests', ['invalid-nested-duplicate-json-reordered']]])
 
 describe('BIDS validation', () => {
   const schemaMap = new Map([
@@ -82,7 +80,7 @@ describe('BIDS validation', () => {
 
     if (tests && tests.length > 0) {
       test.each(tests)('$testname: $explanation ', (test) => {
-        if (shouldRun(name, test.testname, onlyRun)) {
+        if (shouldRun(name, test.testname, runAll, runMap, skipMap)) {
           validate(test)
         } else {
           console.log(`----Skipping ${name}: ${test.testname}`)

tests/converter.spec.js

@@ -218,7 +218,8 @@ describe('HED string conversion', () => {
         return validator(testStrings, expectedResults, expectedIssues)
       })
 
-      it('should handle leading and trailing spaces correctly', () => {
+      // TODO: This test is handled by tokenizer and should be removed.
+      it.skip('(REMOVE)should handle leading and trailing spaces correctly', () => {
         const testStrings = {
           leadingSpace: ' Item/Sound/Environmental-sound/Unique Value',
           trailingSpace: 'Item/Sound/Environmental-sound/Unique Value ',
@@ -234,7 +235,8 @@ describe('HED string conversion', () => {
         return validator(testStrings, expectedResults, expectedIssues)
       })
 
-      it.skip('should strip leading and trailing slashes', () => {
+      // TODO: Remove as these are now errors
+      it.skip('(REMOVE)should strip leading and trailing slashes', () => {
         const testStrings = {
           leadingSingle: '/Event',
           leadingExtension: '/Event/Extension',

tests/event.spec.js

@@ -64,7 +64,7 @@ describe('HED string and event validation', () => {
       const validatorSyntactic = validatorSyntacticBase
 
       //TODO: Remove this test -- the separate test for mismatched parentheses is no longer performed.
-      it.skip('should not have mismatched parentheses', () => {
+      it.skip('(REMOVE) should not have mismatched parentheses', () => {
         const testStrings = {
           extraOpening:
             'Action/Reach/To touch,((Attribute/Object side/Left,Participant/Effect/Body part/Arm),Attribute/Location/Screen/Top/70 px,Attribute/Location/Screen/Left/23 px',
@@ -88,7 +88,7 @@ describe('HED string and event validation', () => {
       })
 
       // TODO: This test is replaced by tokenizer tests and should be removed
-      it.skip('should not have malformed delimiters', () => {
+      it.skip('(REMOVE) should not have malformed delimiters', () => {
         const testStrings = {
           missingOpeningComma:
             'Action/Reach/To touch(Attribute/Object side/Left,Participant/Effect/Body part/Arm),Attribute/Location/Screen/Top/70 px,Attribute/Location/Screen/Left/23 px',
@@ -177,7 +177,8 @@ describe('HED string and event validation', () => {
         validatorSyntactic(testStrings, expectedIssues, (validator) => {})
       })
 
-      it('should not have invalid characters', () => {
+      // TODO: This test will be replaced by invalid character tests based on value classes
+      it.skip('should not have invalid characters', () => {
         const testStrings = {
           openingBrace: 'Attribute/Object side/Left,Participant/Effect{Body part/Arm',
           closingBrace: 'Attribute/Object side/Left,Participant/Effect}/Body part/Arm',
@@ -276,8 +277,8 @@ describe('HED string and event validation', () => {
         )
       }
 
-      // TODO: Fix
-      it.skip('should not contain duplicates', () => {
+      // TODO: Remove test as repeated in bidsTests
+      it.skip('(REMOVE) should not contain duplicates - now in bidsTests', () => {
         const testStrings = {
           noDuplicate: 'Event/Category/Experimental stimulus,Item/Object/Vehicle/Train,Attribute/Visual/Color/Purple',
           legalDuplicate: 'Item/Object/Vehicle/Train,(Item/Object/Vehicle/Train,Event/Category/Experimental stimulus)',
@@ -354,11 +355,11 @@ describe('HED string and event validation', () => {
   })
 
   describe('HED-3G validation', () => {
-    const hedSchemaFile = 'tests/data/HED8.2.0.xml'
+    const hedSchemaFile = 'tests/data/HED8.3.0.xml'
     let hedSchemas
 
     beforeAll(async () => {
-      const spec3 = new SchemaSpec('', '8.2.0', '', hedSchemaFile)
+      const spec3 = new SchemaSpec('', '8.3.0', '', hedSchemaFile)
       const specs = new SchemasSpec().addSchemaSpec(spec3)
       hedSchemas = await buildSchemas(specs)
     })
@@ -405,7 +406,8 @@ describe('HED string and event validation', () => {
     describe('Full HED Strings', () => {
       const validatorSemantic = validatorSemanticBase
 
-      it('properly validate short tags', () => {
+      // TODO: Remove - Units moved to bidsTests rest to stringParser tests
+      it.skip('REMOVE properly validate short tags', () => {
         const testStrings = {
           simple: 'Car',
           groupAndValues: '(Train/Maglev,Age/15,RGB-red/0.5),Operate',
@@ -422,7 +424,7 @@ describe('HED string and event validation', () => {
             generateIssue('unitClassInvalidUnit', {
               tag: 'Time-value/20 cm',
               unitClassUnits: legalTimeUnits.sort().join(','),
-            }),
+            }), //Now in bidsTests
           ],
           duplicateSame: [
             generateIssue('duplicateTag', {
@@ -451,6 +453,7 @@ describe('HED string and event validation', () => {
         })
       })
 
+      // TODO: The testing of units should be in the stringParser
       it('should not validate strings with short-to-long conversion errors', () => {
         const testStrings = {
           // Duration/20 cm is an obviously invalid tag that should not be caught due to the first error.
@@ -567,7 +570,8 @@ describe('HED string and event validation', () => {
         )
       }
 
-      it('should exist in the schema or be an allowed extension', () => {
+      // TODO: Already covered in stringParserTests -- units still to move down.
+      it.skip('REMOVE should exist in the schema or be an allowed extension', () => {
         const testStrings = {
           takesValue: 'Time-value/3 ms',
           full: 'Left-side-of',
@@ -613,6 +617,7 @@ describe('HED string and event validation', () => {
         )
       })
 
+      // TODO: Wait to generate tests until detection moved to stringParser.
       it('should have a proper unit when required', () => {
         const testStrings = {
           correctUnit: 'Time-value/3 ms',
@@ -635,8 +640,6 @@ describe('HED string and event validation', () => {
           /*properTime: 'Clockface/08:30',
         invalidTime: 'Clockface/54:54',*/
         }
-        const legalTimeUnits = ['s', 'second', 'day', 'minute', 'hour']
-        // const legalClockTimeUnits = ['hour:min', 'hour:min:sec']
         const legalFrequencyUnits = ['Hz', 'hertz']
         const legalSpeedUnits = ['m-per-s', 'kph', 'mph']
         const expectedIssues = {
@@ -656,7 +659,7 @@ describe('HED string and event validation', () => {
           incorrectUnit: [
             generateIssue('unitClassInvalidUnit', {
               tag: testStrings.incorrectUnit,
-              unitClassUnits: legalTimeUnits.sort().join(','),
+              unitClassUnits: 'day,hour,minute,month,s,second,year',
             }),
           ],
           incorrectNonNumericValue: [
@@ -685,7 +688,7 @@ describe('HED string and event validation', () => {
           incorrectNonSIUnitModifier: [
             generateIssue('unitClassInvalidUnit', {
               tag: testStrings.incorrectNonSIUnitModifier,
-              unitClassUnits: legalTimeUnits.sort().join(','),
+              unitClassUnits: 'day,hour,minute,month,s,second,year',
             }),
           ],
           incorrectNonSIUnitSymbolModifier: [
@@ -717,6 +720,7 @@ describe('HED string and event validation', () => {
         )
       })
 
+      // TODO: BIDS sidecar validation does not detect missing definitions (under definition-tests in bidsTests)
       it('should not contain undefined definitions', () => {
         const testDefinitions = {
           greenTriangle: '(Definition/GreenTriangleDefinition/#, (RGB-green/#, Triangle))',
@@ -743,7 +747,8 @@ describe('HED string and event validation', () => {
         })
       })
 
-      it('should have a child when required', () => {
+      // TODO: The requireChild seems to be hardcoded somewhere as stringParser doesn't require it.
+      it.skip('(INCORRECT REWRITE) should have a child when required', () => {
         const testStrings = {
           noRequiredChild: 'Red',
           hasRequiredChild: 'Label/Blah',

tests/schemaBuildTests.spec.js

@@ -12,11 +12,9 @@ import { schemaBuildTestData } from './testData/schemaBuildTests.data'
 import { Schemas } from '../schema/containers'
 
 // Ability to select individual tests to run
+const skipMap = new Map()
 const runAll = true
-let onlyRun = new Map([])
-if (!runAll) {
-  onlyRun = new Map([['invalid-schemas', ['lazy-partnered-with conflicting-tags-build']]])
-}
+const runMap = new Map([['invalid-schemas', ['lazy-partnered-with conflicting-tags-build']]])
 
 describe('Schema build validation', () => {
   beforeAll(async () => {})
@@ -62,7 +60,7 @@ describe('Schema build validation', () => {
 
     if (tests && tests.length > 0) {
       test.each(tests)('$testname: $explanation ', async (test) => {
-        if (shouldRun(name, test.testname, onlyRun)) {
+        if (shouldRun(name, test.testname, runAll, runMap, skipMap)) {
           await testSchema(test)
         } else {
           console.log(`----Skipping ${name}: ${test.testname}`)

tests/schemaSpecTests.spec.js

@@ -9,11 +9,9 @@ import { shouldRun } from './testUtilities'
 import { schemaSpecTestData } from './testData/schemaBuildTests.data'
 
 // Ability to select individual tests to run
+const skipMap = new Map()
 const runAll = true
-let onlyRun = new Map()
-if (!runAll) {
-  onlyRun = new Map([])
-}
+const runMap = new Map([])
 
 describe('Schema validation', () => {
   beforeAll(async () => {})
@@ -51,7 +49,7 @@ describe('Schema validation', () => {
 
     if (tests && tests.length > 0) {
       test.each(tests)('$testname: $explanation ', (test) => {
-        if (shouldRun(name, test.testname, onlyRun)) {
+        if (shouldRun(name, test.testname, runAll, runMap, skipMap)) {
           validateSpec(test)
         } else {
           console.log(`----Skipping ${name}: ${test.testname}`)

tests/stringParserTests.spec.js

@@ -7,15 +7,13 @@ import { BidsHedIssue } from '../bids/types/issues'
 import { buildSchemas } from '../schema/init'
 import { SchemaSpec, SchemasSpec } from '../schema/specs'
 
-import { conversionTestData } from './testData/stringParserTests.data'
+import { parseTestData } from './testData/stringParserTests.data'
 import { shouldRun, getHedString } from './testUtilities'
 
 // Ability to select individual tests to run
+const skipMap = new Map()
 const runAll = true
-let onlyRun = new Map()
-if (!runAll) {
-  onlyRun = new Map([['invalid-long-to-short', ['single-level-extension-already-a-tag']]])
-}
+const runMap = new Map([['valid-mixed-groups', []]])
 
 describe('Parse HED string tests', () => {
   const schemaMap = new Map([
@@ -36,34 +34,46 @@ describe('Parse HED string tests', () => {
 
   afterAll(() => {})
 
-  describe.each(conversionTestData)('$name : $description', ({ name, tests }) => {
-    const convert = function (test) {
+  describe.each(parseTestData)('$name : $description', ({ name, tests }) => {
+    const testConvert = function (test) {
       const status = test.errors.length === 0 ? 'Expect pass' : 'Expect fail'
       const header = `[${test.testname} (${status})]`
       const thisSchema = schemaMap.get(test.schemaVersion)
       assert.isDefined(thisSchema, `header: ${test.schemaVersion} is not available in test ${test.name}`)
 
       // Parse the string before converting
       const [parsedString, errorIssues, warningIssues] = getHedString(test.stringIn, thisSchema)
+
+      // Check for errors
       assert.deepStrictEqual(
         errorIssues,
         test.errors,
         `${header}: expected ${errorIssues} errors but received ${test.errors}\n`,
       )
+      // Check if warning match
       assert.deepStrictEqual(
         warningIssues,
         test.warnings,
         `${header}: expected ${warningIssues} warnings but received ${test.warnings}\n`,
       )
-
-      let resultString = null
-      if (parsedString !== null) {
-        resultString = parsedString.format(test.operation === 'toLong')
+      if (parsedString === null) {
+        return
       }
+
+      // Check the conversion to long
+      const longString = parsedString.format(true)
+      assert.strictEqual(
+        longString,
+        test.stringLong,
+        `${header}: expected ${test.stringLong} but received ${longString}`,
+      )
+
+      // Check the conversion to short
+      const shortString = parsedString.format(false)
       assert.strictEqual(
-        resultString,
-        test.stringOut,
-        `${header} [${test.operation}]: expected ${test.stringOut} but received ${resultString}`,
+        shortString,
+        test.stringShort,
+        `${header}: expected ${test.stringShort} but received ${shortString}`,
       )
     }
 
@@ -73,8 +83,8 @@ describe('Parse HED string tests', () => {
 
     if (tests && tests.length > 0) {
       test.each(tests)('$testname: $explanation ', (test) => {
-        if (shouldRun(name, test.testname, onlyRun)) {
-          convert(test)
+        if (shouldRun(name, test.testname, runAll, runMap, skipMap)) {
+          testConvert(test)
         } else {
           console.log(`----Skipping ${name}: ${test.testname}`)
         }