Add Node V16 build and compatibility

[devinea]

• Adapt code due to API change in node 16.
  https://github.com/nodejs/node/pull/37216/files#diff-11c13307cb334211e0d11f4e7e43bdc0a34e437779a63a6ff645e62448d973d2R1053-R1055

• Only write files to disk if UX_DEBUG environment variable is set when running unit tests.

[tobiasqueck]

@devinea instead of using a new library to delete the debug folder before all tests, could we instead change the debugging approach and only commit the folders to the file system if debug mode is enabled?
It would remove the requirement to delete anything (btw I think that was also the initial implementation of template tests by @ullasholla )

[devinea]

@tobiasqueck that is sort of a separate issue. In debug mode the dir stills needs to be deleted. Fs api's have a breaking change so it is easier to switch to fs-extra or rimraf for consistent API across versions.

[tobiasqueck]

While I agree that this is kind of a separate issue, I saw it while reviewing this code and, I don't think it is a good idea to fix an issue that only exists because of another deeper issue: we have unit tests that rely on the file system. To be precise, not the tests itself rely on it but test preparation method that do things that are not related to the tests but for a very different problem: generating artifacts to manually review them.

In debug mode the dir stills needs to be deleted

No, there shouldn't be the need to delete anything because the tests will not write anything to the filestystem and they also don't rely on the file system. Only the debug mode would commit/write data to the fs, so that a developer can review. Under a normal (not debug) run nothing is ever written.

[devinea]

@tobiasqueck - Based on our discussion I have added the UX_DEBUG env to control writing the test-output or not via fs-commit. Since the test folders should be deleted anyway when UX_DEBUG is either true or false I've kept fs-extra in place for that.

[tobiasqueck]

• checked that it works even if the test-output folder doesn't exist
• reviewed the tests and verified that no data is written to the file system if one is not running in debug mode
• looked at general coding style and all looks good to me
• I did not check that data is written to the filesystem if it runs in debug mode

[ullasholla]

@tobiasqueck that is sort of a separate issue. In debug mode the dir stills needs to be deleted. Fs api's have a breaking change so it is easier to switch to fs-extra or rimraf for consistent API across versions.

@devinea , what's the breaking change in rmdirSync again? Curious because this might affect tools-suite too.

[ullasholla]

Changes look good, thanks.

There's one minor formatting change required (there's a suggestion that can be committed directly, if you're ok with it).

There's a comment about abstracting common code in the tests. Up to you how you handle this.

I'm curious about the breaking change in the fs API though. This might have an impact on our other code. Mind posting a link or two about it please?

[devinea]

@ullasholla

There's one minor formatting change required (there's a suggestion that can be committed directly, if you're ok with it).

Applied. Thanks.

There's a comment about abstracting common code in the tests. Up to you how you handle this.

Yeah makes sense. Can look at that in another PR.

I'm curious about the breaking change in the fs API though. This might have an impact on our other code. Mind posting a link or two about it please?

See here https://github.com/nodejs/node/pull/37216/files#diff-11c13307cb334211e0d11f4e7e43bdc0a34e437779a63a6ff645e62448d973d2R1053-R1055

[tobiasqueck]

Approved again. Only formatting changes since my initial review.

[ullasholla]

Thanks @devinea, looks good.