jaredwray / flat-cache

A stupidly simple key/value storage using files to persist the data
MIT License
165 stars 30 forks source link

test: switch to vitest #91

Closed uncenter closed 6 months ago

uncenter commented 8 months ago

This PR migrates the project from mocha + chai + c8 to vitest. The tests are passing right now but there is an issue with an error in the circular JSON test on lines 357-371... still working that out. See image below:

Screenshot 2024-01-01 at 23 18 09 (Code)

You can run tests with npm run vitest command.

uncenter commented 8 months ago

Still haven't figured out the circular error but everything else is working...

uncenter commented 8 months ago

This doesn't seem to be at all related to vitest and just some other issue that snuck past previous testing? Trying running the following code in the root directory of the flat-cache repository:

// index.js
const fc = require('./src/cache.js');
const utils = require('./src/utils');
const path = require('path');

const cache = fc.load('someId');

const data = {
  foo: 'foo',
  bar: 'bar',
};

data.circular = data;

cache.setKey('some-key', data);
cache.getKey('some-key');

cache.save();
utils.readJSON(path.resolve(__dirname, './.cache/someId'))['some-key'];

You'll get the same error I was seeing through Vitest. I haven't changed any internals, just the tests, which is something to note.

jaredwray commented 8 months ago

@uncenter - to you plan to do this? If not, we can wait on it as most likely I would move this to AVA.

uncenter commented 8 months ago

This should have been good to merge, but I'm not sure why that circular JSON thing is an issue. It isn't related to Vitest at all, as displayed in my previous reply. I think no matter what testing framework you choose to adopt, this circular JSON error is something we should figure out first since it seems to be entirely separate.

uncenter commented 7 months ago

I could tell it to --dangerouslyIgnoreUnhandledErrors for now? Not sure if that is a solution you would feel comfortable with. Ideally we should fix the root cause of the error first.

jaredwray commented 6 months ago

@uncenter - going to close this and will ping you as we are going to be migrating this in the coming months to cacheable as a package and will rewrite it in typescript with async.

uncenter commented 6 months ago

This doesn't seem to be at all related to vitest and just some other issue that snuck past previous testing? Trying running the following code in the root directory of the flat-cache repository:

// index.js
const fc = require('./src/cache.js');
const utils = require('./src/utils');
const path = require('path');

const cache = fc.load('someId');

const data = {
  foo: 'foo',
  bar: 'bar',
};

data.circular = data;

cache.setKey('some-key', data);
cache.getKey('some-key');

cache.save();
utils.readJSON(path.resolve(__dirname, './.cache/someId'))['some-key'];

You'll get the same error I was seeing through Vitest. I haven't changed any internals, just the tests, which is something to note.

Should I create an issue for this? And wouldn't upgrading the testing environment be a nice thing to do before refactoring/rewriting to ensure functionality is the same and improve DX while you're at it?

jaredwray commented 6 months ago

Nope as we will be doing a full rewrite and port it to this mono repo in the upcoming months. We will also bump the version as we will most likely move to async and await for the calls.

uncenter commented 6 months ago

Nope as we will be doing a full rewrite and port it to this mono repo in the upcoming months. We will also bump the version as we will most likely move to async and await for the calls.

Sweet! Good luck with the rewrite.

jaredwray commented 6 months ago

Yep! If you want to help I will ping you when we start it. Just let me know