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

Cypress component tests report eslint erorrs #600

Open
braughtg opened this issue Dec 20, 2022 · 19 comments · May be fixed by #634
Open

Cypress component tests report eslint erorrs #600

braughtg opened this issue Dec 20, 2022 · 19 comments · May be fixed by #634
Assignees
Labels
bug Something isn't working testing Issues related to tests.

Comments

@braughtg
Copy link
Member

braughtg commented Dec 20, 2022

When running the cypress component tests lots of eslint errors are reported. These appear to be a configuration issue rather than an actual issue with the code as they mostly report undefined variables or functions in cypress (e.g. 'cy' is not defined). But the tests themselves run perfectly fine.

Steps to Reproduce

  1. cd to farmdata2_modules
  2. run ./test_runner.bash ct --spec **/Error*.spec.comp.js
  3. observe the errors at the top of the ouput.

Also note that the test itself passes. This suggests maybe that eslint is running in a different context than the cypress tests themselves. It is worth further note that if the tests are run in the Cypress GUI there is no reporting of eslint errors at all.

Simply disabling the running of eslint before the component tests would be an acceptable solution.

@braughtg braughtg added bug Something isn't working testing Issues related to tests. labels Dec 20, 2022
@FutzMonitor
Copy link
Collaborator

I had to run this inside of the /FarmData2/farmdata2 directory since the project has been reorganized since the creation of this issue.

This is the result of the output:
image

@braughtg
Copy link
Member Author

braughtg commented Mar 3, 2023

Interesting I get the following:
image

Is it possible that this is related to your work on #601 where you may have removed the ErrorBannerComponent?

Try the following command instead... It will generate similar errors:

  • ./test_runner.bash ct --spec **/Date*.spec.comp.js

@FutzMonitor
Copy link
Collaborator

FutzMonitor commented Mar 3, 2023

Is it possible that this is related to your work on #601 where you may have removed the ErrorBannerComponent?

Try the following command instead... It will generate similar errors:

* `./test_runner.bash ct --spec **/Date*.spec.comp.js`

I ran the above command on my laptop using the main branch which still has the ErrorBannerComponent present. I re-ran the command above on my desktop computer when I realized there was a mistake in my previous comment. I forgot to add the forward slash before the word "Error". Correcting this mistake produces the same result you had:

image

Running the ./test_runner.bash ct --spec **/Date*.spec.comp.js produces the following output:

image

@FutzMonitor
Copy link
Collaborator

I can try taking a look into this issue.

@FutzMonitor
Copy link
Collaborator

FutzMonitor commented Mar 6, 2023

I'm taking a look at the eslint documentation and there's a page on disabling eslint for all JavaScript files [link]. However, this solution isn't working. The errors continue to occur. Alternatively, I could add the lines /* eslint-disable */ to the top of every single **.spec.comp.js along with the component files themselves. I think this is a pretty unsatisfactory solution however. It would be a lot nicer to simply disable eslint in one location instead of having to worry about multiple location. Do you have any tips of some other way to disable the eslint?

@braughtg
Copy link
Member Author

braughtg commented Mar 6, 2023

I don't think disabling es-lint in the files is the right solution. The plan eventually will be to have es-lint running in VSCodium and enforcing a style on all of the code so that it is uniform across the project. If it is possible to disable this just for the running of the tests via cypress in the test container that would probably be fine.

Maybe there is a flag that you can pass to the Cypress test runner? Then that could be added to the call in the test_runner.bash script? Another alternative, is that maybe something else needs to be installed into the fd2test project that is used to run the tests in the dev container?

I haven't had much time to investigate this.

@FutzMonitor
Copy link
Collaborator

Where exactly is the eslint installed? I'm looking around the project and the fd2dev's docker files, and I can't quite pinpoint exactly where eslint is being introduced.

@braughtg
Copy link
Member Author

braughtg commented Mar 6, 2023

I think it must be being installed as part of the normal cypress instal in the dev container. I think I would start by looking at how to run cypress from the command line and then looking at the test_runner.bash script. That will give you a feel for how this comes together.

Note that the dockerfile for the dev container also creates the fd2test project in the fd2dev home directory. That is really where the testing is running from. The farmdata2 directory is mounted into that fd2test directory to make all of the .spec files available to Cypress.

@FutzMonitor
Copy link
Collaborator

I found it within the fd2test project inside the fd2dev container. I'll see what I can do to disable eslint just for Cypress testing.

image

@braughtg
Copy link
Member Author

braughtg commented Mar 7, 2023

That looks promising. You may be able to find something that works by making changes in the dev container directly for testing. Ultimately, we'll want to figure out what the issue is and correct it in the Dockerfile and associated files in the docker/dev directory so that when the container is built it will work correctly out of the box.

An alternative to disabling eslint for the tests would be to figure out how to make it work with Cypress in the dev container. It seems as if eslint is intended to work correctly within Cypress, but for some reason it is not finding the Cypress or cy variables. So it is possible that installing Cypress or eslint differently would also solve the problem.

@FutzMonitor
Copy link
Collaborator

It was mentioned that instead of running the RUN command in order to create the .eslintignore file and its contents, it would be more consistent to make use of the COPY command. However, I'm reading documentation on this command [source] and it seems to copy pre-existing files on the local machine into the project. Does that mean I should make the .eslintignore a file in the project and simple have the .gitignore not make it appear in the repository? Having it available on the local machine is useful because people who attempt to run Cypress tests via the command line on their host machine versus the dev container will experience the very issues I'm attempting to resolve.

@braughtg
Copy link
Member Author

I would suggest creating a file in docker/dev called eslintignore that contains the content that you want .eslintignore in the container to have. Then add a line to copy this fie into the container to the Dockerfile near where it uses COPY to copy the other files from docker/dev (lines 169-182). Notice that those lines copy the files and also rename them to include the . at the start. This is so that these files are not hidden files in the repo (no .) but are hidden files, as they are supposed to be, in the container.

@FutzMonitor FutzMonitor linked a pull request Mar 27, 2023 that will close this issue
@FutzMonitor
Copy link
Collaborator

I noticed that the webpack-dev-server runs some linting tests as well. So it might be worthwhile exploring if the linting tests are happening there and being pipelined with the cypress tests.

@braughtg
Copy link
Member Author

braughtg commented Apr 25, 2023

I noticed that the webpack-dev-server runs some linting tests as well.

Sounds promising!

There seems to be some information here that might be helpful:

@FutzMonitor
Copy link
Collaborator

The webpack module isn't installed by us but by Cypress as a dependency for testing. As a result, the config file people are often referring to isn't present. The following modules are present when webpack is installed.

  • webpack
  • webpack-bundle-analyzer
  • webpack-chain
  • webpack-dev-server
  • webpack-merge
  • webpack-sources
  • webpack-virtual-modules
    It'll be worthwhile to install it separately to have better control of its configurations to have better control of its install. I'll get on it soon.

@braughtg
Copy link
Member Author

braughtg commented May 2, 2023

  • It'll be worthwhile to install it separately to have better control of its configurations to have better control of its install.

Installing webpack prior to Cypress might work as long as we ensure that the installed version is compatible with the dependency required by Cypress. One way to test this would be to uninstall webpack that was installed by Cypress and then install it again manually. If everything still works, then this seems like it could be a viable approach.

I also wonder if there is a config tool somewhere for webpack? Maybe there is something here: https://webpack.js.org/configuration/

@FutzMonitor
Copy link
Collaborator

I uninstalled webpack and ran the test runner, but it worked as if nothing was uninstalled. I ran the test runner in both the Farmdata2/farmdata2 directory and the fd2test/farmdata2 directory and neither stopped functioning after uninstalling webpack. I tried reinstalling it without any scripts with the following command: npm install webpack --no-save --no-scripts and re-ran the test runner in both of those directories and it worked again and the errors continued to be outputted.

@braughtg
Copy link
Member Author

I uninstalled webpack and ran the test runner, but it worked as if nothing was uninstalled.

Are you saying here that the linting errors were also reported when you ran the tests after uninstalling webpack?

@FutzMonitor
Copy link
Collaborator

Are you saying here that the linting errors were also reported when you ran the tests after uninstalling webpack?

Yes, the linting errors were reported despite webpack being uninstalled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working testing Issues related to tests.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants