Simpletest.module doxygen fixes

Thanks! This doesn't seem quite right to me though:

a)

+ *   The test id.

Sorry... "id" is a psychological term (ego, superego, id). You need to use "ID" to refer to an identifier.

b)
PHPUnit
Is that right, or is it PhpUnit? We normally use CamelCaseEvenForAcronymsLikePhp, but of course we didn't invent this... so I'm just asking which is correct?

c)

- * Inserts the parsed phpunit results into the simpletest table.
+ * Inserts the parsed PHPUnit results into the Simpletest table.

The table is most likely called "simpletest", right? Or ... actually is it a different table?

d)

- * @param array $phpunit_results
- *   An array of test results returned from simpletest_phpunit_xml_to_rows.
+ * @param array[] $phpunit_results
+ *   An array of test results returned from simpletest_phpunit_xml_to_rows().

Ummmm... This parameter is expecting an array of arrays? Probably needs more explanation.

e)

/**
- * Read the error log and report any errors as assertion failures.
+ * Reads the error log and report any errors as assertion failures.

report -> reports

f)

- * @return
+ * @return array[]
  *   An array of tests keyed with the groups specified in each of the tests
  *   getInfo() method and then keyed by the test class. An example of the array

As long as you are fixing things, that should be "tests'" at the end of the line there.

g)

+ * @param $filename string
+ *   The name of the file, including the path.

string should be before $filename

h)

+ * Converts PHPUnit's junit xml output to an array.
...
+ *   Path to the PHPUnit xml file.

xml -> XML... and should JUnit have some capitalization?

i)

+ * Converts a PHPUnit test case result to a Simpletest result row.
...
+ *   An array containing the Simpletest result row.

See (c).

j)

+ *
+ * @throws \RuntimeException
  */
 function simpletest_phpunit_get_available_tests($module = NULL) {

Some explanation of when/why this exception is thrown would be good? Just saying the exception is RuntimeException doesn't tell us anything useful, as the class name is pretty generic here.

Just some explanations. Will re-roll later.

s that right, or is it PhpUnit? We normally use CamelCaseEvenForAcronymsLikePhp, but of course we didn't invent this... so I'm just asking which is correct?

The official name is PHPUnit. We should stick to that capitalization, unless when it's used in class or property names. These occurrences are in human-readable text.

The table is most likely called "simpletest", right? Or ... actually is it a different table?

If it's a table name, it should be written in curly brackets. To me this reads "into Simpletest's table", whatever name it may have.

Ummmm... This parameter is expecting an array of arrays? Probably needs more explanation.

Nothing weird about that. It's what it is. The exact array structure should be documented at simpletest_phpunit_xml_to_rows(). I don't know the structure myself, just that it is an array of arrays.

OK mostly... But on the table/module name...
"Inserts the parsed PHPUnit results into the Simpletest table."
The module is not called "Simpletest" and the table is not called "Simpletest", so either way this is wrong.

If you want to refer to the module's capitalized name, it should be "the Testing module" and if you want to make it clear it isn't a particular table, there should be an apostrophe like "into the Testing module's tables" or something like that.

Good, I think most of the capitalization stuff is fixed up, and the documentation seems clear enough... A few mishaps still:

a) It looks like the proper capitalization of JUnit is http://junit.org/ "JUnit".

b) "XML" should always be written that way, not "xml".

c) Still a "Simpletest" here:

+  // Insert the results of the PHPUnit test run into the database so the results
+  // are displayed along with Simpletest's results.

Should probably say "... are displayed along with WebTestBase results" or something like that?

d)

+ * @return array[]
+ *   An array of tests keyed with the groups specified in each of the tests'
+ *   getInfo() methods and then keyed by the test classes. An example of the
+ *   array structure is provided below.
  *
  *   @code
  *     $groups['Block'] => array(

If you want that @code block to appear as part of the @return on api.drupal.org, you need to get rid of the blank line.

Still a "Simpletest" here:
+ // Insert the results of the PHPUnit test run into the database so the results
+ // are displayed along with Simpletest's results.

Should probably say "... are displayed along with WebTestBase results" or something like that?

Regardless of the name of our testing module, everybody still refers to those type of tests as Simpletest tests, because that's what that framework is a port of. Besides providing a testing framework, the module also functions as our generic testing UI, which is why the name changed. WebTestBase is not a test framework name, but one of three of four base classes that Drupal provides for the Simpletest testing framework.

OK, thanks for the explanation -- works for me.

There's an "avoid commit conflicts" issue whose patch touches this file, so I'm going to wait on committing this:
#1996238: Replace hook_library_info() by *.libraries.yml file
There may not be a direct conflict with this patch, but sometimes people working on Big Issues like that use a sandbox workflow that makes any changes to the same files problematic.

The patch at #8 no longer applies. Re-rolling the patch from #8.

The rerolled patch in #11 is not quite the same as the patch in #8.

In simpletest_test_get_all() docs -- missing the ' at the end of one line in the @return.

I don't see any other differences, but can we get that ' back please?

Here we go..

Thanks!

1. +++ b/core/modules/simpletest/simpletest.module
   @@ -569,7 +577,19 @@ function simpletest_classloader_register() {
   + * @param int $lines
   + *   The number of line in the file.
   

   lines

2. +++ b/core/modules/simpletest/simpletest.module
   @@ -569,7 +577,19 @@ function simpletest_classloader_register() {
   + * @param string $type
   + *   "text", "binary", or "binary-text".
   

   Use "The .." pattern and made those a list of examples. It's also optional, so added (optional).

3. +++ b/core/modules/simpletest/simpletest.module
   @@ -788,15 +812,16 @@ function simpletest_phpunit_get_available_tests($module = NULL) {
   + *   An array of rows in a format that can be inserted into the
   + *   simpletest results table.
   

   wrapping this to 80 chars.

   but I wonder if it should specify the table name and use {} like #6 did else where.

how about:

- *   An array of rows in a format that can be inserted into the simpletest
- *   results table.
+ *   The results as array of rows in a format that can be inserted into
+ *   {simpletest}.

patch that does that.
the rest looks good to me.

RE #15, we could fix a *lot* more stuff in simpletest.module... Thanks for those updates! All the changes that are in the latest patch look good to me too.

Thanks again, everyone! Committed to 8.x.