Skip to content

Conversation

@navDhammu
Copy link
Collaborator

@navDhammu navDhammu commented Nov 13, 2024

Description

This is a prequel PR which blocks #3523. Even if the team doesn't agree on that one then this is still relevant because updating to the latest version is probably a good idea anyways.

Currently, our jest version is still at 26.4 which is an old and unmaintained version. There are some peer dependencies in this version that are incompatible with the workspaces feature of pnpm, and not just pnpm but npm as well after some further testing.

Updating to the latest version isn't straightforward because there were breaking changes in jest v27. The main breaking change was that using both done callback and returning a promise is no longer supported - see old blog post. I guess the reason we're still using the old version because there is too much effort to do the migration - there's over 50 test files with thousands of lines of code.

So I used babel to write a custom code migration script with the help of this Babel Plugin Handbook. The script is complicated but to explain simply - it loops through each test file, parses the code into a tree like data structure, and then applies the necessary changes for the jest migration, and rewrites the modified code back to the test file.

Click to view the script used
import parser from '@babel/parser';
import _traverse from '@babel/traverse';
import t from '@babel/types';
import recast from 'recast';
import { readFile, writeFile, readdir } from 'fs/promises';
import path from 'path';
import { Binding } from './binding.js';

const traverse = _traverse.default;

const files = await readdir('tests');

for (const file of files) {
  if (!file.endsWith('test.js')) continue;

  const filePath = path.join('tests', file);
  const code = await readFile(filePath, { encoding: 'utf-8' });
  const ast = recast.parse(code, {
    parser,
  });

  const funcs = {};
  const doneParamFuncs = {};

  try {
    traverse(ast, {
      FunctionDeclaration(p) {
        const {
          params,
          body: { body },
          id: { name },
        } = p.node;

        const lastStmt = body[body.length - 1];
        if (
          !params.length ||
          params[params.length - 1].name !== 'callback' ||
          t.isReturnStatement(lastStmt) ||
          lastStmt.expression?.callee?.property?.name !== 'end'
        )
          return;

        params.pop();
        body[body.length - 1] = t.returnStatement(lastStmt.expression.callee.object);
        funcs[name] = true;
      },

      Function(p) {
        const doneParamIndex = p.node.params.findIndex((param) => param?.name === 'done');
        if (doneParamIndex > -1) {
          p.node.async = true;
          p.get(`params.${doneParamIndex}`).remove();
          doneParamFuncs[p.node.id?.name] = doneParamIndex;
        }
      },

      ExpressionStatement(p) {
        const expression = p.node.expression.callee?.object;
        if (expression?.callee?.name === 'expect' && expression.arguments[0]?.name === 'err') {
          p.remove();
        }
      },

      VariableDeclaration(p) {
        if (p.node.declarations[0]?.id?.name?.endsWith('AsPromise')) {
          p.remove();
        }
      },

      CallExpression(p) {
        const { callee, arguments: args } = p.node;

        if (callee.name === 'done') {
          p.remove();
          return;
        }

        if (callee.name?.endsWith('AsPromise')) {
          callee.name = callee.name.slice(0, callee.name.length - 'AsPromise'.length);
          return;
        }

        const doneArgIndex = args.findIndex((arg) => arg?.name === 'done');
        if (doneArgIndex === -1) return;
        p.get('arguments')[doneArgIndex].remove();
        p.replaceWith(t.awaitExpression(p.node));
      },

      IfStatement(p) {
        if (p.node.test.name === 'err') p.remove();
      },
    });

    traverse(ast, {
      CallExpression: {
        enter(p) {
          const { name } = p.node.callee;

          if (doneParamFuncs[name]) {
            p.remove(p.get('arguments')[doneParamFuncs[name]]);
          }

          if (funcs[name]) {
            const args = p.node.arguments;
            const lastArg = args[args.length - 1];
            if (lastArg?.type !== 'ArrowFunctionExpression') return;

            args.pop();

            const lastParamName = lastArg.params[lastArg.params.length - 1]?.name;

            let has = p.scope.hasBinding(lastParamName);
            if (lastParamName && !has) {
              p.scope.bindings[lastParamName] = new Binding({
                identifier: t.identifier(lastParamName),
                scope: p.scope,
                path: null,
                kind: 'const',
              });
            }

            const stmts = [];
            stmts.push(
              lastParamName
                ? t.variableDeclaration('const', [
                    t.variableDeclarator(t.identifier(lastParamName), t.awaitExpression(p.node)),
                  ])
                : t.expressionStatement(t.awaitExpression(p.node)),
            );

            for (let i = 0; i < lastArg.body.body.length; i++) {
              const node = lastArg.body.body[i];
              if (node.type === 'VariableDeclaration') {
                const { id } = node.declarations[0];
                const varName = t.isArrayPattern(id) ? id.elements[0].name : id.name;
                if (p.scope.hasBinding(varName)) {
                  has = true;
                } else {
                  p.scope.bindings[varName] = new Binding({
                    identifier: id,
                    scope: p.scope,
                    path: null,
                    kind: node.kind,
                  });
                }
              }
              stmts.push(node);
            }

            p.replaceWithMultiple(has ? t.blockStatement(stmts) : stmts);

            p.skip();
            return;
          }
        },
      },
    });

    writeFile(filePath, recast.print(ast).code);
  } catch (e) {
    console.log('error in file: ', file);
    console.log(e);
  }
}

It automatically makes all the required changes (except a few lines of code which I manually changed) including:

  • Removing all instances of done from function parameters, as well done() function calls.
  • Rewriting request function declarations to use promise instead of callback. And then wherever they are called, make the necessary changes like using await and replacing the callback.

Example - refactoring postCropRequest function declaration

// Before
function postCropRequest(data, { user_id = newOwner.user_id, farm_id = farm.farm_id }, callback) {
  chai
    .request(server)
    .post('/crop')
    .set('Content-Type', 'application/json')
    .set('user_id', user_id)
    .set('farm_id', farm_id)
    .send(data)
    .end(callback);
}

// After
function postCropRequest(data, { user_id = newOwner.user_id, farm_id = farm.farm_id }) {
  return chai
    .request(server)
    .post('/crop')
    .set('Content-Type', 'application/json')
    .set('user_id', user_id)
    .set('farm_id', farm_id)
    .send(data)
}

Example - refactoring postCropRequest function call

// Before
postCropRequest(crop, {}, (err, res) => {
  expect(res.status).toBe(400);
  expect(JSON.parse(res.error.text).error.data.crop_common_name[0].keyword).toBe('required');
  done();
});

// After
const res = await postCropRequest(crop, {}) 
expect(res.status).toBe(400);
expect(JSON.parse(res.error.text).error.data.crop_common_name[0].keyword).toBe('required')

@navDhammu navDhammu marked this pull request as ready for review November 14, 2024 18:26
@navDhammu navDhammu requested review from a team as code owners November 14, 2024 18:26
@navDhammu navDhammu requested review from antsgar and kathyavini and removed request for a team November 14, 2024 18:26
@navDhammu navDhammu added dependencies Pull requests that update a dependency file and removed dependencies Pull requests that update a dependency file labels Nov 14, 2024
@antsgar
Copy link
Collaborator

antsgar commented Mar 28, 2025

@navDhammu so sorry, this one has been open forever and we never got to reviewing it! I apologize. Do you think after merging conflicts it'd still be relevant? If so I can take a look next week!

@navDhammu
Copy link
Collaborator Author

@antsgar No worries. I think as long as all tests written after this PR was opened, were written using the correct syntax for the newer jest versions then it should be ok.

@navDhammu
Copy link
Collaborator Author

@antsgar After merging it looks like the newer tests written for animals are failing and they would need to be refactored to use the proper done callback / promise syntax for this jest version. Would you like me to look into this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants