Use ts-jest instead of babel directly / run prettier during github actions / fix prettier issues

[huntharo]

Changes

• Remove direct use of babel
• Add ts-jest
• Split lint out of the test script
• Invoke the individual commands in GitHub Actions so we can tell quickly what failed
  • lint
  • unit tests
  • xmlint test
  • prettier 🆕 (there are failures in 4 files - fixed)
• Remove file specifiers from test script so we can pass params like npm run test -- tests/sitemap-index.test.ts --watch when testing single files
• Replace deprecated toThrowError with toThrow
• Fix typescript complaints in test files
• Convert utils.js to typescript
• Change some test requires to imports

[derduher]

I appreciate this but you might want to wait just a bit longer. I've got a branch cooking that does a lot of updates including dropping older versions of node and updating to 18 as the min version

[huntharo]

I appreciate this but you might want to wait just a bit longer. I've got a branch cooking that does a lot of updates including dropping older versions of node and updating to 18 as the min version

It's ok, I can pull some hunks out of this if parts of it work.

[huntharo]

Hmm... the coverage report seems to have broken

[derduher]

Alright updated master with all the package bumps (except eslint 9 because ts parser is a bit behind)

[huntharo]

@derduher - This is ready for review now. Runs prettier during the github actions too, which found that some errors had crept in.

[derduher]

Overall I like this but can you fix the failing test for xmllint (it shows up when installed)? Also it'd be nice to get the coverage report unbroken. Feel free to replace whatever is reporting coverage.

● Console

    console.log
      [
        Error: Command failed: xmllint --schema /Users/patrickweygand/projects/schema/all.xsd --noout ./tests/mocks/cli-urls.json.xml
        warning: failed to load external entity "/Users/patrickweygand/projects/schema/all.xsd"
        Schemas parser error : Failed to locate the main schema resource at '/Users/patrickweygand/projects/schema/all.xsd'.
        WXS schema /Users/patrickweygand/projects/schema/all.xsd failed to compile
        
            at genericNodeError (node:internal/errors:984:15)
            at wrappedFn (node:internal/errors:538:14)
            at ChildProcess.exithandler (node:child_process:422:12)
            at ChildProcess.emit (node:events:519:28)
            at maybeClose (node:internal/child_process:1105:16)
            at Socket.<anonymous> (node:internal/child_process:457:11)
            at Socket.emit (node:events:519:28)
            at Pipe.<anonymous> (node:net:338:12) {
          code: 5,
          killed: false,
          signal: null,
          cmd: 'xmllint --schema /Users/patrickweygand/projects/schema/all.xsd --noout ./tests/mocks/cli-urls.json.xml'
        },
        'warning: failed to load external entity "/Users/patrickweygand/projects/schema/all.xsd"\n' +
          "Schemas parser error : Failed to locate the main schema resource at '/Users/patrickweygand/projects/schema/all.xsd'.\n" +
          'WXS schema /Users/patrickweygand/projects/schema/all.xsd failed to compile\n'
      ]

      at Object.<anonymous> (xmllint.test.ts:31:25)

  ● xmllint › resolves when complete

    Command failed: xmllint --schema /Users/patrickweygand/projects/schema/all.xsd --noout ./tests/mocks/cli-urls.json.xml
    warning: failed to load external entity "/Users/patrickweygand/projects/schema/all.xsd"
    Schemas parser error : Failed to locate the main schema resource at '/Users/patrickweygand/projects/schema/all.xsd'.
    WXS schema /Users/patrickweygand/projects/schema/all.xsd failed to compile



  ● xmllint › resolves when complete

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      30 |         console.log(e);
      31 |         expect(true).toBe(false);
    > 32 |       }
         |        ^
      33 |     } else {
      34 |       console.warn('skipping xmlLint test, not installed');
      35 |       expect(true).toBe(true);

      at Object.<anonymous> (xmllint.test.ts:32:30)

[huntharo]

Overall I like this but can you fix the failing test for xmllint (it shows up when installed)? Also it'd be nice to get the coverage report unbroken. Feel free to replace whatever is reporting coverage.

How the heck did I miss that?

The issue is from:

• The change from import { xmlLint } ../dist/index imports to reference the import { xmlLint } from '../lib/xmllint';
• resolve(__dirname, '..', '..', 'schema', 'all.xsd'), in xmllint.ts

Changing '..', '..' to '..' makes the test pass.

But... I think this is pretty fragile.

I think we need to make a couple changes here, but I'm not positive what I want to do yet.

Things I have in mind:

• The path needs to be '..', '..' when using the compiled JS since the file is at ./dist/lib/xmllint.js
• The path needs to be '..' when using the source TS because the file is at ./lib/xmllint.ts
• Change xmlLint() to accept a parameter that tells it where the schema file is - The tests can then pass the correct path relative to their unchanging source directory and the CLI could do the same
• OR: copy the schema file into ./dist/schema so everything can just use '..' and work

[derduher]

I'm in favor of the last two with a default path. I think the only reason dist is included in the published packages as a subfolder is I was too lazy to move it just for the ts compile on publish.

npm publish --dry-run

npm notice 10.7kB CHANGELOG.md
npm notice 3.4kB CODE_OF_CONDUCT.md
npm notice 1.1kB LICENSE
npm notice 9.4kB README.md
npm notice 14.6kB api.md
npm notice 31B dist/cli.d.ts
npm notice 5.2kB dist/cli.js
npm notice 1.1kB dist/index.d.ts
npm notice 4.6kB dist/index.js
npm notice 3.0kB dist/lib/errors.d.ts
npm notice 10.2kB dist/lib/errors.js
npm notice 2.3kB dist/lib/sitemap-index-parser.d.ts
npm notice 6.2kB dist/lib/sitemap-index-parser.js
npm notice 5.8kB dist/lib/sitemap-index-stream.d.ts
npm notice 8.8kB dist/lib/sitemap-index-stream.js
npm notice 989B dist/lib/sitemap-item-stream.d.ts
npm notice 9.6kB dist/lib/sitemap-item-stream.js
npm notice 2.3kB dist/lib/sitemap-parser.d.ts
npm notice 21.1kB dist/lib/sitemap-parser.js
npm notice 1.3kB dist/lib/sitemap-simple.d.ts
npm notice 3.8kB dist/lib/sitemap-simple.js
npm notice 2.4kB dist/lib/sitemap-stream.d.ts
npm notice 5.2kB dist/lib/sitemap-stream.js
npm notice 655B dist/lib/sitemap-xml.d.ts
npm notice 1.4kB dist/lib/sitemap-xml.js
npm notice 12.4kB dist/lib/types.d.ts
npm notice 6.0kB dist/lib/types.js
npm notice 2.3kB dist/lib/utils.d.ts
npm notice 12.8kB dist/lib/utils.js
npm notice 319B dist/lib/xmllint.d.ts
npm notice 1.4kB dist/lib/xmllint.js
npm notice 353B jest.config.cjs.bk
npm notice 353B jest.config.js
npm notice 2.4kB package.json
npm notice 822B schema/all.xsd
npm notice 4.5kB schema/sitemap.xsd

[huntharo]

@derduher I believe this is ready for review now. The test has been fixed. I fixed an issue with the lint script too that was missing the tests directory when run during actions or locally.

[derduher]

Any idea what is going on with coverage tests?

[derduher]

figured it out. will fix in master but the collectCoverageFrom option is relative to rootDir in jest.config.js