civictechindex / CTI-website-frontend

Join a worldwide movement to catalog 
every open source 
civic tech project.
https://civictechindex.org
MIT License
31 stars 29 forks source link

Fix vulnerabilities 1103 #1119

Closed fyliu closed 2 years ago

fyliu commented 2 years ago

1103

bruceplai commented 2 years ago

@fyliu It appears cypress tests are failing. You can run the following in your local dev environment to see which tests are affected.

npx cypress run
bruceplai commented 2 years ago

One of the errors from the automated cypress tests:

1) Projects Page (Search Projects)
       loads:
     Error: The following error originated from your application code, not from Cypress.

  > useLocation() may be used only in the context of a <Router> component.

When Cypress detects uncaught errors originating from your application it will automatically fail the current test.

This behavior is configurable, and you can choose to turn this off by listening to the `uncaught:exception` event.

https://on.cypress.io/uncaught-exception-from-application
      at invariant (http://localhost:3000/static/js/bundle.js:187389:20)
      at useLocation (http://localhost:3000/static/js/bundle.js:187686:35)
      at Projects (http://localhost:3000/static/js/bundle.js:53597:105)
      at renderWithHooks (http://localhost:3000/static/js/bundle.js:173188:22)
      at mountIndeterminateComponent (http://localhost:3000/static/js/bundle.js:175800:17)
      at beginWork (http://localhost:3000/static/js/bundle.js:176878:20)
      at HTMLUnknownElement.callCallback (http://localhost:3000/static/js/bundle.js:158581:18)
      at Object.invokeGuardedCallbackDev (http://localhost:3000/static/js/bundle.js:158630:20)
      at invokeGuardedCallback (http://localhost:3000/static/js/bundle.js:158683:35)
      at beginWork$1 (http://localhost:3000/static/js/bundle.js:181444:11)
fyliu commented 2 years ago

Looks like the tests were already failing in main. I disabled the 4 failing tests in main, then applied my changes and all the non-failing tests still passed.

I will look for the merge that broke the tests. The 4 tests are always failing, unlike a couple of them that only sometimes fail.

Also, updating npm packages that resulted in upgrading material-ui was probably a bad idea for this pull request. I will make one without that part.

fyliu commented 2 years ago

I see now that the automated testing is run against Electron 93 and the failures I was seeing was against Firefox. So main is not failing any tests.

I redid the changes to skip the extra npm update that broke the app because of material-ui changes

codeclimate[bot] commented 2 years ago

Code Climate has analyzed commit 1e0ec017 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 80.9% (0.1% change).

View more on Code Climate.

bruceplai commented 2 years ago

Overall looks good, but I'd like to have other team members test this PR as well.