gtsop / babel-jest-boost

🐠 πŸƒ πŸš€ - Brings tree-shaking to Jest, speeding up your test runs, using Babel
https://www.npmjs.com/package/@gtsopanoglou/babel-jest-boost
GNU Affero General Public License v3.0
10 stars 1 forks source link

feature: improve mock replace #4

Closed maloguertin closed 14 hours ago

maloguertin commented 2 months ago

🎯 Changes

This PR aims to better support mock path rewrites:

Previously this code:

// library/index.js
export { target } from "./library";

// library/library.js
export function target() {}

// global/index.js
export * from "../library";

// code to transform
import { target } from './test_tree/global';
jest.mock('./test_tree/global');

would become:

_getJestObj().mock(".../test_tree/global/index.js");

import { target } from ".../test_tree/library/library.js";

this means that if you were to do something like expect(target).toHaveBeenCalled() the test could not pass because here target is not a mock because the mock is on a lower level than the import.

it also takes care of mocking all exports of the barrel file:

// library/index.js
export { target } from "./library";
export { otherTarget } from "./otherLibrary"

// library/library.js
export function target() {}

// library/otherLibrary.js
export function otherTarget() {}

// global/index.js
export * from "../library";

// code to transform
import { target } from './test_tree/global';
jest.mock('./test_tree/global');

will become:

_getJestObj().mock(".../test_tree/library/library.js");
_getJestObj().mock(".../test_tree/library/otherLibrary.js");
import { target } from ".../test_tree/library/library.js";

βœ… Checklist

πŸ”„ Depends on

The changes from https://github.com/gtsop/babel-jest-boost/pull/3 are included in this PR because tests would fail otherwise

gtsop-d commented 2 months ago

Hey @maloguertin please check https://github.com/gtsop/babel-jest-boost/pull/3#issuecomment-2276587093 first

gtsop-d commented 2 months ago

On the code of this PR:

image

  1. The jest-utils/ directory has code i have copied from jest to help me manipulate it's config object, all the code that we write for this plugin should be in the plugin/ directory. So the code you put in jest-utils should live somewhere within plugin
  2. The spec/ directory contains what we would call "acceptance" or "end-2-end" tests, whereas each file within plugin/ will contain the "unit"-level tests, having the test file right next to the tested code. You've added test cases in the spec/ directory which indeed awesome, but you've completely removed any unit-level tests in the new code you wrote. I assume the new code you wrote wasn't very compatible with the old tests and that's why the deletion. I think it will be necessary to add some unit tests there especially to test this direct rerun logic you've put (which i haven't managed to understand yet, i need to study it a bit)
  3. We're gonna need a rebase here again to remove the noise in the diff from your previous PR :D
maloguertin commented 2 months ago

The jest-utils/ directory has code i have copied from jest to help me manipulate it's config object, all the code that we write for this plugin should be in the plugin/ directory. So the code you put in jest-utils should live somewhere within plugin

Oh yes sorry about that, the reason I moved it was that the code is no longer really a codemod since it doesn't directly modify the path. It also unfortunately became harder to test as an input/ouput test because of that.

I will try to find a better way structure that code.

From what I was seeing, the difference between the codemod test and the spec test for mocks we're simply to isolate only the replacements of mocks with more than 2 arguments.

I could move most of the code from the call expression into it's own codemod so that it is testable as a unit test?

gtsop-d commented 2 months ago

I could move most of the code from the call expression into it's own codemod so that it is testable as a unit test?

yeap sounds good, please make sure to check https://github.com/gtsop/babel-jest-boost/pull/3#issuecomment-2276587093 first

maloguertin commented 2 months ago

I could move most of the code from the call expression into it's own codemod so that it is testable as a unit test?

yeap sounds good, please make sure to check #3 (comment) first

https://github.com/gtsop/babel-jest-boost/pull/5

If I move the code to a new file that means I have to export bjbResolve, traceSpecifierOrigin, isPathWhitelisted

I could move them to their own file but since they rely on module scoped variables being initialized I would have to refactor a little bit.

Two possible solutions:

// cachedHelpers.js

let modulePaths = null;
let moduleNameMapper = null;
let ignoreNodeModules = false;
let importWhiteList = [];

const initializeHelpers = (config) => {
  modulePaths = config.modulePaths;
  moduleNameMapper = config.moduleNameMapper;
  ignoreNodeModules = config.ignoreNodeModules;
  importWhiteList = config.importWhiteList;
}

const isPathWhitelisted = withCache(function actualIsPathWhitelisted(path) {
  return matchAnyRegex(importWhiteList, path);
});

const resolve = withCache(function resolveWithWhitelist(path, basedir) {
  try {
    if (isPathWhitelisted(path)) {
      return path;
    }

    const result = resolve(path, basedir, moduleNameMapper, modulePaths);

    if (ignoreNodeModules && result.includes("/node_modules/")) {
      return path;
    }

    return result;
  } catch (e) {
    console.log("failed to resolve", e.message);
    return null;
  }
});

const tracer = new Tracer(bjbResolve);

const traceSpecifierOrigin = withCache(
  function actualTraceSpecifierOrigin(specifierName, codeFilePath) {
    return tracer.traceOriginalExport(specifierName, codeFilePath);
  },
);

module.exports = { resolve, traceSpecifierOrigin, isPathWhiteListed } 

or I had an initialize higher order function to these wrappers and pass the helpers as a parameter to the codemod?

gtsop commented 2 months ago

@maloguertin I think this is a good opportunity for all those functions to be extracted, they shouldn't live in the index.js anyway. Your first solution sounds good since it has the least amount of change to the current structure.

Please make sure you hit a rebase first to add new code on top of all the previous commits you did

Note: don't name them cached helpers, the caching is subject to change

gtsop commented 2 months ago

hey @maloguertin , i realize this is holiday season, i just need to check if in general you are going to follow up on this MR at all or have given up

maloguertin commented 1 month ago

I'm sorry, I've been busy I'll try put some work on it!

maloguertin commented 1 month ago

@gtsop I'm having trouble extracting tests for the codemode for mocks. Your original implementation was not resolving any files so the tests we're straightforward. If I extract most of the CallExpression into the code mode then the tests have to be very similar to the jest-mock-rewrites-spec.js and king of lose their value.

gtsop-d commented 14 hours ago

@maloguertin This breaks a single test case in my codebase and I am trying to resolve it.

gtsop-d commented 14 hours ago

@maloguertin I cannot get it fixed quickly, but I want your code in. I will merge this MR and use a no-boost on my test that breaks.

gtsop commented 10 hours ago

Released under v0.1.18