fix: fixed tests for node

[mszmida]

This change fixes compatibility of tests with Node and Jest. Current Node LTS version is v20.13.0. On GitHub Actions we are using:

[nobkd]

Would recommend something like

import { join, dirname } from 'node:path'
import { fileURLToPath } from 'node:url'

fileURLToPath(dirname(import.meta.url))

ref: https://github.com/nuejs/create-nue/pull/43/commits/86034d2176c90757fcf5affbcfee3073d8a911e9

---

• Use named imports. At least that's done so almost everywhere currently
• Build the paths with join() or so, so it also works on Windows
• Define fs.readFile(filepath, 'utf-8') to get a string and not a buffer (https://nodejs.org/api/fs.html#filehandlereadfileoptions)

You can copy relatively much from e.g. here: https://github.com/nuejs/nue/blob/master/packages/nuekit/test/nuekit.test.js

---

Maybe something like the following incomplete example:

import { join, dirname } from 'node:path'
import { fileURLToPath } from 'node:url'
import { promises as fs } from 'node:fs'

import { createKit } from '../src/nuekit.js'


const __dirname = fileURLToPath(dirname(import.meta.url))

// temporary directory
const root = join(__dirname, 'page-router-test')
const dist = join(root, '.dist')
const distDev = join(dist, 'dev')

// setup and teardown
beforeAll(async () => {
  await fs.rm(dist, { recursive: true, force: true })

  const nue = await createKit({ root }) // maybe not working (path problems...), but needed if not tested from root of monorepo but for this module only(?)
  await nue.build()
})

// ...

function read(filePath) {
  return fs.readFile(join(distDev, filePath), 'utf-8')
}

---

There might still be jest not defined and/or wrong environment errors...

[nobkd]

Maybe avoid jest.restoreAllMocks() if feasible and instead of jest.spyOn just spyOn and restore the mock at the end of the test manually mocked.mockRestore()? Just some ideas.

EDIT: (absolute) paths might be a problem with jest/... not sure, why though

[mszmida]

@nobkd Thank you for the review and hints! Currently the tests are failing because esbuild path resolution works differently on node:

export async function getBuilder(is_esbuild) {
  const actual_cwd = process.env.ACTUAL_CWD || process.cwd()
  try {
    return is_esbuild ? await import(resolve('esbuild', `file://${actual_cwd}/`)) : Bun
  } catch {
    throw 'Bundler not found. Please use Bun or install esbuild'
  }
}

I will take a look on that tomorrow.

[tipiirai]

@mszmida looks like some checks were not succesful, so I'm hesitant to merge this

[nobkd]

Yes it's not running properly currently, because something (jest or...) handles path resolution differently, I think...

---

Hidden as off-topic

@tipiirai The tests currently don't run automatically when new contributors make a pull request. I don't know if there is a way to let them run automatically regardless, or not. But it would be great, if you would let those tests run before merging next time. I would have allowed the run, but I don't seem to have enough permissions to do so, so you'll have to do that.. :)

[tipiirai]

@nobkd I added a maintainer role for you.

Can read, clone, and push to this repository. They can also manage issues, pull requests, and some repository settings.

Should be enough!

[fritx]

Hi, I'd suggest to keep the compatibility with older node.js as to give the best user ( developer)-experience as we can. Also a breaking-change which could be avoided is not good for any library. Maybe a polyfill for __dirname after a type check?

~~Also, sorry that I did not involve node20 in the CI as node20 has such big breaking changes.~~ ~~As I suggest, running both node18 & node20 in the CI is a choice to keep best compatibility for nuejs developers.~~ Sorry, the actual reason was not of node18 or node20, I totally forgot that it was actually because that __dirname is not supported in type=module mode. cc @mszmida

[fritx]

+1 to avoid jest* stuffs as much as possible, as nuejs testing is based on bun test, and I introduced jest* to ensure the compatibility to users with node environment. I tried to keep the jest* stuffs minimal and compatible with the original testing APIs in bun.

[nobkd]

I made my own branch, and made these changes, which imo made the code prettier and more portable. There is still the issue with the missing environment, but when that is added, there are esbuild-bundler-missing-issues for whatever reason. (probably wrong dynamic import or so) :shrug: (EDIT: more likely that the environment change is responsible for that... :face_exhaling:)

Edit: My current full changes (have thrown out jest happy-dom env again, and importing necessary happy-dom globals directly in the test file... Now I have problems with jest global missing (see test run) and some process hanging behind :shrug:)

[mszmida]

Hello Guys,

I am sorry for the lack of any sign of life on my part. I was completely occupied with the affairs of my own company.

@nobkd Thanks for changing the PR status to draft. I suppose to do it at the beginning.

@tipiirai Fully understand that you didn't want to merge this 👍 I will continue to work on it later this week. I will make all tests on Windows on my local to debug what is actually going on here.

[nobkd]

I now have a working implementation for the tests (#284). Sorry, if you put any more time into this. I'll close this now, but please comment on my implementation, if you see potential for improvement! See real test run: https://github.com/nobkd/nue/actions/runs/9982211641