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

changes to issue #1558 regarding failed Jest test cases #1577

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

binkmywink
Copy link

I was able to make all the tests work by rewriting the CSSTransform file and updating the Jest version in package.json (this can also be done by running npm update jest. I used babel npm install babel-jest --save-dev to reconfigure some files, as I believe that was causing some of the tests to fail. The main goal was to configure Jest for ECMAScript Modules, as the issue came from trying to import d3 using ECMAScript Module syntax (import * as d3 from 'd3') in the Histogram test. Using the minified version of d3 resolved this issue. In addition to updating snapshots before running backend tests, I was able to compile all the tests successfully.

@zqian zqian changed the title changes to issue 1558 regarding failed Jest test cases changes to issue #1558 regarding failed Jest test cases Apr 26, 2024
@zqian zqian requested a review from pushyamig April 26, 2024 16:02
@@ -1,4 +1,4 @@
import * as d3 from 'd3'
import * as d3 from 'd3/dist/d3.min'
Copy link
Contributor

@pushyamig pushyamig Apr 26, 2024

Choose a reason for hiding this comment

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

I don't feel we import things from dist folder. This change effects the View. why are you doing this?

Copy link
Contributor

Choose a reason for hiding this comment

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

None of the imports in the project we use dist in path while importing. This needs to be changed.

["@babel/preset-react",{"runtime": "automatic"}]]
[
"@babel/preset-env",
{
Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to keep this format same as before. What was the reason for re-formatting this?

@@ -7,7 +7,7 @@ const mylaGlobals = mylaGlobalsEl ? JSON.parse(mylaGlobalsEl.textContent) : {}

let cspNonce
const cspMetaEl = document.querySelector('meta[name="csp-nonce"]')
if (cspMetaEl !== null & cspMetaEl.hasAttribute('content')) {
if (cspMetaEl !== null && cspMetaEl.hasAttribute('content')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Undo the change here as I don't feel this needed any change

// // The output is always the same.
// return 'cssTransform';
// }
// };
Copy link
Contributor

Choose a reason for hiding this comment

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

You should always remove commented code , if not used

@zqian zqian linked an issue Apr 26, 2024 that may be closed by this pull request
@pushyamig
Copy link
Contributor

pushyamig commented Apr 26, 2024

Can you email me or share your Uniqname? we are keeping track of students from EECS course who are contributing to our repo

Copy link
Contributor

@pushyamig pushyamig left a comment

Choose a reason for hiding this comment

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

I have commented on code and I felt needs change

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.

Fix Jest Frontend tests
2 participants