-
Notifications
You must be signed in to change notification settings - Fork 13
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
Setup and Write E2E tests #75
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mehul0810,
Thanks a lot for working on this and raising the PR. Added some comments/notes on PR to check and discuss could you please help to check on that once? Please feel free to reach out if you need any further information on comments.
Thank you.
- name: Build | ||
run: npm run build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we don't have a build
script in package.json, we need to remove this.
"url": "https://github.com/10up/wp-newrelic/issues" | ||
}, | ||
"engines": { | ||
"node": ">=12.13.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe update this to v16 to match what we have in .nvmrc
.
"files": [ | ||
"readme.txt", | ||
"README.md", | ||
"CHANGELOG.md", | ||
"composer.json", | ||
"distributor.php", | ||
".github/workflows/*", | ||
".gitattributes", | ||
"assets/img/*", | ||
"dist/**/*", | ||
"includes/**/*", | ||
"lang/**/*", | ||
"templates/**/*", | ||
"vendor/yahnis-elsts/plugin-update-checker/**/*" | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noticed some files which are not related to this repo. eg: "distributor.php", "vendor/yahnis-elsts/plugin-update-checker/**/*" etc... Could you please remove those from the list? Thanks
@@ -0,0 +1,62 @@ | |||
<?php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if we need this plugin in the test environment, as I don't see its usage anywhere in tests. Could you please remove it if are not using it? Thanks
@@ -0,0 +1,110 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this report file is accidentally committed to the repo, Could you Please add the tests/cypress/reports
folder to .gitignore
? It contains test report files and shouldn't be pushed to the repo.
then | ||
echo "Multisite already initialized" | ||
else | ||
echo "Converting to multisite" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a multisite environment here? Not very much familiar with the plugin but based on functionality it seems we don't require a multisite test setup here. Please let me know if I am missing anything here. Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iamdharmesh might be best to have you work on updates here and merge this in as you're able
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closing in favour of #83 |
Description of the Change
Closes #55
How to test the Change
Changelog Entry
Credits
Props @mehul0810
Checklist: