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

Added E2E test for ParseHookNames #22868

Closed
wants to merge 4 commits into from

Conversation

akgupta0777
Copy link
Contributor

@akgupta0777 akgupta0777 commented Dec 4, 2021

Summary

Adding an E2E test for the parseHooksNames feature.
Linked Issue #22646

What the test is trying to do?

Added tests for every component which have hooks.
How it works:

  • Goes to the components through inspecting in dev tools.
  • Clicks on parse hooks names toggle and wait for parsing to complete.
  • Validates the hook names with predefined hooks array.

How did you test this change?

Ran yarn test:e2e and all tests are passing.

What changes have you done till now?

  • Fixed problem of previous test breaking randomly on hashed selectors.
  • Refactored the code for previous tests.
  • Improved Code quality
  • Added test for every Hook component
  • Assigned IDs to various components accordingly to make it easier for writing tests.
  • Solved problem of tests running simultaneously now they are running sequentially.

Quick Demo

E2e.tests.react.mp4

@akgupta0777 akgupta0777 marked this pull request as draft December 4, 2021 18:12
@akgupta0777 akgupta0777 marked this pull request as ready for review December 7, 2021 16:59
@akgupta0777 akgupta0777 changed the title [WIP]: Adding E2E test for ParseHookNames Added E2E test for ParseHookNames Dec 7, 2021
@akgupta0777
Copy link
Contributor Author

akgupta0777 commented Dec 10, 2021

I confirm that all tests are working on NodeJS v14 and above.
I ran tests on 14.17.0, 15.10.0, 16.13.0 and they are working fine

Whoops! react-devtools-inline's webpack watch process fails on the latest Node v17.2.0 therefore can't able to run tests for now on that version.

@akgupta0777
Copy link
Contributor Author

For Context on Node v17.2.0
I ran yarn start on react-devtools-inline package.
Here is the error stack

Abhay@DESKTOP-I04BUTC MINGW64 /e/Github/react/packages/react-devtools-inline (E2E-Tests)
$ yarn start
yarn run v1.22.15
$ cross-env NODE_ENV=development webpack --config webpack.config.js --watch

webpack is watching the files…

node:internal/crypto/hash:67
  this[kHandle] = new _Hash(algorithm, xofLen);
                  ^

Error: error:0308010C:digital envelope routines::unsupported
    at new Hash (node:internal/crypto/hash:67:19)
    at Object.createHash (node:crypto:130:10)
    at module.exports (E:\Github\react\packages\react-devtools-inline\node_modules\webpack\lib\util\createHash.js:135:53)       
    at NormalModule._initBuildHash (E:\Github\react\packages\react-devtools-inline\node_modules\webpack\lib\NormalModule.js:417:16)
    at handleParseError (E:\Github\react\packages\react-devtools-inline\node_modules\webpack\lib\NormalModule.js:471:10)        
    at E:\Github\react\packages\react-devtools-inline\node_modules\webpack\lib\NormalModule.js:503:5
    at E:\Github\react\packages\react-devtools-inline\node_modules\webpack\lib\NormalModule.js:358:12
    at E:\Github\react\packages\react-devtools-inline\node_modules\loader-runner\lib\LoaderRunner.js:373:3
    at iterateNormalLoaders (E:\Github\react\packages\react-devtools-inline\node_modules\loader-runner\lib\LoaderRunner.js:214:10)
    at iterateNormalLoaders (E:\Github\react\packages\react-devtools-inline\node_modules\loader-runner\lib\LoaderRunner.js:221:10)
    at E:\Github\react\packages\react-devtools-inline\node_modules\loader-runner\lib\LoaderRunner.js:236:3
    at context.callback (E:\Github\react\packages\react-devtools-inline\node_modules\loader-runner\lib\LoaderRunner.js:111:13)  
    at E:\Github\react\packages\react-devtools-inline\node_modules\babel-loader\lib\index.js:59:71 {
  opensslErrorStack: [ 'error:03000086:digital envelope routines::initialization error' ],
  library: 'digital envelope routines',
  reason: 'unsupported',
  code: 'ERR_OSSL_EVP_UNSUPPORTED'
}

Node.js v17.2.0
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@akgupta0777
Copy link
Contributor Author

This Happened because node.js 17 uses OpenSSL3 which have changed code for initialization context of md hashing family (including md4 hashing algorithm) that's why hashing breaks
But I think for testing purposes we don't need node 17 urgently we can work on node 14 to 16 versions for testing.

Apart from that if we have to support Node17 for tests in future then we also have to upgrade to webpack 5 because it will not be available in webpack 4. See Comment

@akgupta0777
Copy link
Contributor Author

@bvaughn need help ?

@bvaughn
Copy link
Contributor

bvaughn commented Dec 15, 2021

You might want to rebase this PR on top of #22968 once it lands.

I pulled in the span[class^=Value] change to that PR so that the pre-existing tests are all actually fixed, but I think we should step back and re-think that selector approach before writing more tests.

@bvaughn
Copy link
Contributor

bvaughn commented Dec 15, 2021

Hey @akgupta0777 😄 Thanks for your help on this stuff!

I wonder if you would mind holding off making additional changes to the e2e tests for now? I'd like to take a step back and give the high level approach a little more thought (before we start adding more individual tests). I'm concerned it's too brittle with the way we're approaching it.

@akgupta0777
Copy link
Contributor Author

@bvaughn Sure I have no problem with it. Try whatever you want 😀

@bvaughn
Copy link
Contributor

bvaughn commented Dec 16, 2021

Took a pass this afternoon with #22978

@bvaughn
Copy link
Contributor

bvaughn commented Dec 21, 2021

Thank you for this PR 🙇🏼

We decided to try a different direction (#22978) built on top of the test selector API.

If it's okay with you, we can just close this PR

@akgupta0777
Copy link
Contributor Author

Yeah I have no problem 🙂. I learned a lot about E2E testing in the entire process and that's enough. Closing now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants