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

[MWPW-133606] Aside block automated scripts #151

Merged

Conversation

chadsunvice
Copy link
Contributor

@chadsunvice chadsunvice commented Aug 14, 2023

PR raised in scope of ticket [MWPW-133606]: Aside Block: Create Nala Automation Test script .

Copy link
Collaborator

@amauch-adobe amauch-adobe left a comment

Choose a reason for hiding this comment

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

see my comments above.

features/milo/aside.block.spec.js Outdated Show resolved Hide resolved
features/milo/aside.block.spec.js Outdated Show resolved Hide resolved
features/milo/aside.block.spec.js Show resolved Hide resolved
tests/milo/aside.block.test.js Show resolved Hide resolved
@chadsunvice
Copy link
Contributor Author

see my comments above.

Updated!

Copy link
Contributor

@skumar09 skumar09 left a comment

Choose a reason for hiding this comment

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

@chadsunvice: PR looks good; just following a few points

  • do we want to add CSS / attributes validations, Or do you want to take care later?

tests/milo/aside.block.test.js Outdated Show resolved Hide resolved
@chadsunvice
Copy link
Contributor Author

@chadsunvice: PR looks good; just following a few points

  • do we want to add CSS / attributes validations, Or do you want to take care later?

I talked to Narcis and also got some feedback from our Devs that there are still Aside changes happening so this will mean most of the checks will become brittle. I would wait for development to stabilise and then take some conclusions on this.

Also we feel CSS validation isn't something that will change, and attributes are already tested via the selectors that have them in their composition. So I would say wait on this.

Copy link
Collaborator

@amauch-adobe amauch-adobe left a comment

Choose a reason for hiding this comment

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

Approved.

@chadsunvice chadsunvice merged commit 971fa94 into adobecom:main Aug 22, 2023
2 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.

4 participants