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 testids in wrangler part-2 #1239

Merged
merged 4 commits into from
Aug 22, 2024
Merged

add testids in wrangler part-2 #1239

merged 4 commits into from
Aug 22, 2024

Conversation

GnsP
Copy link
Collaborator

@GnsP GnsP commented Aug 21, 2024

Add data-testids for wrangler directives

Description

Adds data-testid attributes for all of the wrangler column directives.

Also adds the dev-dependency yaml-jest. It's required by jest (unit test runner) to be able to import the testids.yaml file. yaml-jest is a transformer that takes a yaml file and transforms it into a js module consumable by jest.

PR Type

  • Bug Fix
  • Feature
  • Build Fix
  • Testing
  • General Improvement
  • Cherry Pick

Links

Jira: CDAP-21039

Test Plan

NA

Screenshots

NA

@GnsP GnsP changed the title add testids for wrangler part-2 add testids in wrangler part-2 Aug 21, 2024
@@ -22,9 +22,12 @@ import classnames from 'classnames';
import uuidV4 from 'uuid/v4';
import { Observable } from 'rxjs/Observable';
import intersection from 'lodash/intersection';
import { getDataTestid } from '../../../testids/TestidsProvider';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: This should probably be an absolute path throughout. There's not a strong relationship between the location of each component file and the testids functionality.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will update in all of the files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Replaced all of these relative imports with the following

LC_CTYPE=C sed -i '' -E "s~\'.*\/testids/TestidsProvider\'~\'@cdap-ui\/testids\/TestidsProvider\'~g" $(find app/cdap -type f)

"One sed to rule them all,
one sed to find them,
one sed to bring them all,
and in the files replace them."

@@ -154,7 +154,8 @@
"url-loader": "3.0.0",
"webpack": "4.41.2",
"webpack-cli": "3.3.10",
"webpack-livereload-plugin": "2.2.0"
"webpack-livereload-plugin": "2.2.0",
"yaml-jest": "^1.2.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit : Are we planning to use directly in any test files as JSON? Please clarify.
Also, Could you please update in the description about this change too, so that context would be clear when referred later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a dev-dependency required by jest (unit test runner) to be able to import the testids.yaml file. yaml-jest is a transformer that takes a yaml file and transforms it into a js module consumable by jest.

@GnsP GnsP merged commit a3b9bcc into cdapio:develop Aug 22, 2024
3 checks passed
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.

3 participants