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

Add possibility to load config file from cjs file #7614

Merged
merged 7 commits into from
May 3, 2023

Conversation

gigaga
Copy link
Contributor

@gigaga gigaga commented Apr 6, 2023

[closes #6911]

Purpose

The newest NodeJS version requires indicating modules loading framework to use (CommonJS or ESM) by using extension file. .js extension is now for using ESM and .cjs for CommonJS.
Currently, from newest NodeJS, it is not possible to use CommonJS framework for TestCafe global configuration file (.testcaferc.js), because this file has to be named .testcaferc.cjs

Approach

This PR allows loading .testcaferc.cjs as TestCafe global configuration file.

@need-response-app need-response-app bot added the STATE: Need response An issue that requires a response or attention from the team. label Apr 6, 2023
Copy link
Collaborator

@Aleksey28 Aleksey28 left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution.

src/configuration/formats.ts Outdated Show resolved Hide resolved
@gigaga gigaga temporarily deployed to authentication April 7, 2023 15:25 — with GitHub Actions Inactive
@gigaga gigaga temporarily deployed to CI April 10, 2023 05:37 — with GitHub Actions Inactive
@miherlosev
Copy link
Collaborator

@need-response-app need-response-app bot removed the STATE: Need response An issue that requires a response or attention from the team. label Apr 10, 2023
@gigaga gigaga temporarily deployed to authentication April 11, 2023 08:49 — with GitHub Actions Inactive
@gigaga
Copy link
Contributor Author

gigaga commented Apr 11, 2023

@miherlosev

I fixed lint errors ;)

@need-response-app need-response-app bot added the STATE: Need response An issue that requires a response or attention from the team. label Apr 11, 2023
@gigaga gigaga temporarily deployed to CI April 11, 2023 11:01 — with GitHub Actions Inactive
Copy link
Collaborator

@Aleksey28 Aleksey28 left a comment

Choose a reason for hiding this comment

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

Please fix server tests.

@gigaga gigaga temporarily deployed to authentication April 12, 2023 12:14 — with GitHub Actions Inactive
@gigaga
Copy link
Contributor Author

gigaga commented Apr 12, 2023

Please fix server tests.

It's done. However, It is not possible to ensure that the whole of tests is ok because it seems not possible to be executed on local env. Isn't it?

Regards,

@gigaga gigaga temporarily deployed to CI April 13, 2023 03:40 — with GitHub Actions Inactive
@Aleksey28
Copy link
Collaborator

It's possible. I see a lot of excessive formatting changes in the current version of the commit. Please roll back these changes.

@need-response-app need-response-app bot removed the STATE: Need response An issue that requires a response or attention from the team. label Apr 13, 2023
@gigaga
Copy link
Contributor Author

gigaga commented Apr 13, 2023

Hi,

I fixed again lint issues... :(
I disabled my prettier plugin ;)

@need-response-app need-response-app bot added the STATE: Need response An issue that requires a response or attention from the team. label Apr 13, 2023
@gigaga gigaga temporarily deployed to authentication April 13, 2023 09:55 — with GitHub Actions Inactive
@gigaga gigaga temporarily deployed to CI April 14, 2023 01:48 — with GitHub Actions Inactive
@Aleksey28
Copy link
Collaborator

You still have a few ESLint errors. Please fix them.

@gigaga gigaga temporarily deployed to authentication April 21, 2023 12:55 — with GitHub Actions Inactive
@miherlosev miherlosev removed the STATE: Need response An issue that requires a response or attention from the team. label Apr 25, 2023
@gigaga gigaga temporarily deployed to CI April 25, 2023 06:06 — with GitHub Actions Inactive
@miherlosev
Copy link
Collaborator

At present, there is an issue with building a docker image. We will update this thread once it is fixed.

@gigaga
Copy link
Contributor Author

gigaga commented Apr 25, 2023

The last issue is due to an invalid content in Docker image. Indeed, the content of const Extensions = require('../../lib/configuration/formats'); corresponds to the old value (came from original Docker image FROM testcafe/testcafe:$tag ?):

{
  JS_CONFIGURATION_EXTENSION: '.js',
  JSON_CONFIGURATION_EXTENSION: '.json',
  CONFIGURATION_EXTENSIONS: [ '.js', '.json' ]
}

And expected should be

{
  "js": ".js",
  "json": ".json",
  "cjs": ".cjs"
} 

So, the configurations made with createJsTestCafeConfigurationFile or createJSONTestCafeConfigurationFile are invalid because path is undefined.

I don't know how to fix it...

@need-response-app need-response-app bot added the STATE: Need response An issue that requires a response or attention from the team. label Apr 25, 2023
@miherlosev
Copy link
Collaborator

Rebase please.

@need-response-app need-response-app bot removed the STATE: Need response An issue that requires a response or attention from the team. label Apr 27, 2023
@gigaga gigaga temporarily deployed to authentication April 28, 2023 07:34 — with GitHub Actions Inactive
@gigaga
Copy link
Contributor Author

gigaga commented Apr 28, 2023

Hi @miherlosev

  • Before rebasing, I had some tests failed as describing before (old content of Docker image of test/docker/Dockerfile)

  • After rebasing, the build of Docker image test/docker/Dockerfile is failed due to Error: ENOENT: no such file or directory, open './package.json'. I executed npx gulp docker-server-test-run --steps-as-tasks

Regards,

@need-response-app need-response-app bot added the STATE: Need response An issue that requires a response or attention from the team. label Apr 28, 2023
@gigaga gigaga temporarily deployed to CI April 28, 2023 07:53 — with GitHub Actions Inactive
@miherlosev
Copy link
Collaborator

@gigaga

I see that all tests are green. I will ask our technical writers to update the documentation. As soon as it is ready, I will merge this PR.

@need-response-app need-response-app bot removed the STATE: Need response An issue that requires a response or attention from the team. label May 2, 2023
@miherlosev miherlosev enabled auto-merge (squash) May 3, 2023 07:39
@miherlosev miherlosev disabled auto-merge May 3, 2023 07:40
@miherlosev miherlosev merged commit 2e6dee3 into DevExpress:master May 3, 2023
@gigaga
Copy link
Contributor Author

gigaga commented May 3, 2023

;)

@need-response-app need-response-app bot added the STATE: Need response An issue that requires a response or attention from the team. label May 3, 2023
@miherlosev miherlosev removed STATE: Need response An issue that requires a response or attention from the team. Awaiting documentation labels May 4, 2023
@github-actions
Copy link

github-actions bot commented May 5, 2023

Release v2.6.0-rc.1 addresses 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.

Allow .cjs extension for the configuration file
3 participants