Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(config): update config to detect test.ts files instead of files under test dir #194

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

khalatevarun
Copy link
Contributor

Fix: #190

Copy link

vercel bot commented Dec 27, 2024

@khalatevarun is attempting to deploy a commit to the Antiwork Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

vercel bot commented Dec 27, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
shortest ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 29, 2024 9:56am

@slavingia
Copy link
Contributor

Lint and shortest tests are failing

@slavingia
Copy link
Contributor

May also want to version this as a breaking change. Can be minor as small and early

@m2rads
Copy link
Contributor

m2rads commented Dec 29, 2024

Shortest workflow is failing with this error. I am getting a similar issue locally. Another issue that came up is when deleting node_modules and building and installing the package again, some of the packages are not resolving properly. Shown in the last image below.

image

image

@slavingia
Copy link
Contributor

^ our fault? I still think we should remove pnpm and go to npm.

If someone else could replicate that and fix, would be great. Don't think anything in this branch would cause it, and someone else reported that there may be inconsistencies in the node modules based on npx vs using pnpm in places.

@slavingia
Copy link
Contributor

How does this work if there's a set of tests I don't want to run, or a folder I only want to run tests for, temporarily (via CLI)?

@amk-dev
Copy link
Contributor

amk-dev commented Dec 29, 2024

image

I can see a call to getConfig here in bin.ts -> main, before the initialize is called.

and getConfig throws if the globalConfig is not present.

image

but anything changed recently on this logic ?

const debugAI = args.includes("--debug-ai");

try {
const config = getConfig();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like this code is giving the config error. you're calling getConfig before a call to initialize.

@amk-dev
Copy link
Contributor

amk-dev commented Dec 29, 2024

Another issue that came up is when deleting node_modules and building and installing the package again, some of the packages are not resolving properly. Shown in the last image below.

I tried reproducing this, but no luck. here are the steps i took,

  1. removed node_modules from packages/shortest & root node_modules
  2. pnpm i at the workspace root

no issue with the imports. may be you can try reloading vscode and see it it fixs the issue. ( Cmd+Shift+P -> Developer: Relaod Window ), sometimes it happens.

@m2rads
Copy link
Contributor

m2rads commented Dec 30, 2024

Just posting it here again in case you missed it:

You're right. with the new implementation we should first initialize the runner and then initialize the config: This should fix it:

  try {
    const runner = new TestRunner(
      process.cwd(),
      true,
      headless,
      targetUrl,
      debugAI
    );
    await runner.initialize();

    const config = getConfig();
    const testPattern = cliTestPattern || config.testPattern;

    await runner.runTests(testPattern);
  }

Let me know if you need the full code for this.

Another issue that came up is when deleting node_modules and building and installing the package again, some of the packages are not resolving properly. Shown in the last image below.

I tried reproducing this, but no luck. here are the steps i took,

  1. removed node_modules from packages/shortest & root node_modules
  2. pnpm i at the workspace root

no issue with the imports. may be you can try reloading vscode and see it it fixs the issue. ( Cmd+Shift+P -> Developer: Relaod Window ), sometimes it happens.

@slavingia
Copy link
Contributor

Please merge main to include a new caching feature!

@slavingia
Copy link
Contributor

Would be great to merge this soon so I can use this new approach :)

@slavingia
Copy link
Contributor

Would be great to get to this soon. If it's not done by Friday we'll have some internally do it as I think this is quite important!

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.

updating testDir to testPattern
4 participants