-
Notifications
You must be signed in to change notification settings - Fork 105
Update tests.md #352
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
base: main
Are you sure you want to change the base?
Update tests.md #352
Conversation
Successfulsebunya
left a comment
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.
a few suggested changes on grammar and simple principal. Please correct before merging
| # Tests | ||
|
|
||
| The WordPress Hosting Team provides tools for hosting companies to run the WordPress automated tests on their infrastructure to improve compatibility with WordPress. These results can be published them on the [Host Test Result information page](https://make.wordpress.org/hosting/test-results/), to help WordPress' compatibility with hosts as well. | ||
| The WordPress Hosting Team provides tools for hosting companies to run the WordPress automated tests on their infrastructure to improve compatibility with WordPress. These results can be published on the [Host Test Result information page](https://make.wordpress.org/hosting/test-results/), to help with WordPress compatibility with hosts as well. |
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.
adding with is Good but we also have to maintain the ( ' ) to be grammatically correct.
should be: help with WordPress' compatibility with hosts as well.
| ### Runner | ||
|
|
||
| Hosting companies can have several to millions of websites hosted with WordPress, so it's important to make sure their configuration is as compatible as possible with the software. | ||
| Hosting companies can have from several to millions websites hosted with WordPress, so it's important to make sure their configuration is as compatible as possible with the software. |
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.
this is not correct grammar. Add of
Should be:
Hosting companies can have from several to millions of websites hosted with WordPress, so it's important to make sure their configuration is as compatible as possible with the software.
| ### Reporter | ||
|
|
||
| The Runner tests generates a report with the test results related to a bot user (a hosting company), and this captures and displays those test results at the [Host Test Result](https://make.wordpress.org/hosting/test-results/) page. | ||
| The Runner tests generate a report with the test results related to a bot user (a hosting company), and this captures and displays those test results at the [Host Test Result](https://make.wordpress.org/hosting/test-results/) page. |
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 suggest you add 's to add ownership to the page.
could be:
The Runner tests generate a report with the test results related to a bot user (a hosting company), and this captures and displays those test results at the Host Test Result's page.
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.
For this one I don't have a problem approving it as is, but I do agree that it could be more clear that the bot user belongs to hosting companies. For example: "...related to a bot user associated with a web host, and this captures ..."
| There is [full documentation about this tool](https://make.wordpress.org/hosting/test-results-getting-started/). Also, if you want, you can make your test results appear in the [Host Test Results page](https://make.wordpress.org/hosting/test-results/) of WordPress. | ||
|
|
||
| The tool can be run manually or through an automated system like Travis. To see how it works and the purpose of this document, will be shown how to run the tests manually. | ||
| The tool can be run manually or through an automated system like Travis. To see how it works and the purpose of this document will show how to run the tests manually. |
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.
Fuzzy:
The tool can be run manually or through an automated system like Travis. To see how it works and the purpose of this document is to show how to run the tests manually.
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.
Just one grammatical thing to add (2 commas): "To see how it works, and the purpose of this document, is to show how to run the tests manually."
| export WPT_PREPARE_DIR=/home/wptestrunner/wordpress | ||
|
|
||
| # Path to the directory where the WordPress develop checkout can be placed and tests can be run. When running tests in the same environment, set WPT_TEST_DIR to WPT_PREPARE_DIR | ||
| # Path to the directory where the WordPress development checkout can be placed and tests can be run. When running tests in the same environment, set WPT_TEST_DIR to WPT_PREPARE_DIR |
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 am not sure this needs to change since we have a wordpress-develop repo maybe we could instead highlight it.
I second the change if that is not the intended meaning
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.
This is indeed referring to the wordpress-develop repo, but it isn't clear in the text that that is the case. It should probably say something like: "Path to the directory where a WordPress install can be copied from the wordpress-develop repository, and from which tests can be run."
For reference: https://github.com/WordPress/phpunit-test-runner/blob/master/prepare.php#L82
| Now that the environment has been prepared, the next step is to run the tests for the first time. | ||
|
|
||
| [info]The 4 steps have to be executed every time a test is done. The preparation of the environment as well, even if you do not change the configuration.[/info] | ||
| [info]The 4 steps have to be executed every time a test is done. The preparation of the environment, as well, even if you do not change the configuration.[/info] |
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.
not required
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.
This original sentence is unclear, and grammatically incorrect even with commas. It should say something like: "All four steps must be executed every time a test is run. That includes the preparation of the environment by running prepare.php each time, even if no changes to the env configuration have been made."
| `F` → Means that the test has failed. Information about why this happened is displayed at the end. | ||
|
|
||
| `E` → It means that the test has failed due to a PHP error, which can be an error, warning or notice. | ||
| `E` → It means that the test has failed due to a PHP error, which can be an error, warning, or notice. |
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.
, is not required before and, or and other Joining words
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.
Not required, but also not incorrect. Use of an Oxford comma (like the one added here) is becoming more standard in the US. I do not know for sure what the standard is in other countries, but I believe it's the standard in the UK too. Both are grammatically correct, basically.
| The goal? To be error-free and have the green light for the perfect configuration. | ||
|
|
||
| [alert]Some tests may be skipped or there may be tests with some risk. It is normal for errors to occur even with a properly configured environment.[/alert] | ||
| [alert]Some tests may be skipped, or there may be tests with some risk. It is normal for errors to occur even with a properly configured environment.[/alert] |
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.
, not required
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's not very clear what "there may be tests with some risk" means. There isn't any risk to hosting systems by running these tests even if errors occur. The previous author likely meant that the risk is that the tests and report don't complete due to an error. But that is not clear. It could say something like:
"Some tests may be automatically skipped, or errors could prevent the tests from completing."
| Once the user has been created in the system, you'll get an invitation to join via email. Then, you can log into make.wordpress.org/hosting and create an Application Password in Users -> Your Profile. | ||
|
|
||
| To get things reporting properly, place the username for the bot, along with the application password in the .env file, which will look something like this: `export WPT_REPORT_API_KEY='examplehostingcompanybot:ABCD 1234 abcd 4567 EFGH efgh'`. | ||
| To get things reporting properly, place the username for the bot, along with the application password, in the .env file, which will look something like this: `export WPT_REPORT_API_KEY='examplehostingcompanybot:ABCD 1234 abcd 4567 EFGH efgh'`. |
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.
, not required. Sentence needs to continue without breaking
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.
For me the comma makes sense and is more clear. Maybe some others can chime in here.
No description provided.