[TYPO3-core] RFC #10411: sqlparser is not able to parse more than 1 join

[Oliver Klee]

Hi Xavier,

thanks very much for the unit tests! I've some suggestions for
improvement for them (no real review yet, though):

- Please put setUp and tearDown as the first two methods in your
testcase (and the utility method after that) to improve readability.

- In tearDown, please unset($this->fixture) to help the garbage collector.

- cleanSql expects as string as first parameter (according to the
PHPDoc). So you don't need to check for is_string in the function, but
you need to make sure all callers actually provide a string. Some

- In tests, I find it more readable if the expected value and the actual
value are on different lines (and the message on another line):

  $this->assertEquals(
    'foo',
    $this->fixture->bar(),
    'No coffee today ...'
  );

- If the name of a test functions is descriptive enough, the asserts
usually don't need a message.

- I recommend checking the return type of functions in separate tests.
For example, a test like this ...:

+	public function canParseWhereClause() {
+		$parseString = 'uid IN (1,2) AND (starttime < ' . time() . ' OR
cruser_id + 10 < 20)';
+		$where = $this->fixture->parseWhereClause($parseString);
+
+		if (!is_array($where)) {
+			$this->fail($where);
+		}
+		$this->assertTrue(empty($parseString), 'parseString is not empty');
+	}


... could sprout a test parseWhereClauseReturnsArray() {
    ... assertTrue(
       is_array(...)
    )}

- There's no need to make cleanSql static.

- I wonder why cleanSql is needed at all. Care to elaborate on this?


Thanks again for providing these unit tests!


Oliver