Common InsertTest class tests undocumented use case

[djmattyg007]

The common InsertTest class tests an undocumented way of binding values to a query object. The testBindValues() method calls AbstractQuery::bindValues() with a non-associative array. This use case doesn't appear to be documented, and isn't used by any other code. Is this intentional? Does this test even belong in the InsertTest class?

[djmattyg007]

Judging by #134, it looks like non-named placeholders aren't even supported now, so perhaps this test should just be refactored to use an associative array?

[harikt]

@djmattyg007 I am confused. Are you looking at

Aura.SqlQuery/tests/Common/InsertTest.php

Line 71 in b1bcd35

$this->assertInstanceOf('\Aura\SqlQuery\AbstractQuery', $this->query->bindValues(array('bar', 'bar value')));

? or can you link to the test case you are looking at .

Thank you.

[djmattyg007]

Yes, that is what I'm looking at.

[harikt]

@djmattyg007 looking into that test case, it is only a test for the bindValues is returning an instance. So even if it is not associative it doesn't have any problem to me.

The interface doesn't even care to send an exception if they are wrong.

Aura.SqlQuery/src/QueryInterface.php

Line 65 in b1bcd35

public function bindValues(array $bind_values);

So there is nothing wrong.

If you want to correct to make it associative it is not a problem also. Send a PR.

Thank you.

[djmattyg007]

I think that test case and the one below it should be moved to a different class altogether. They're testing methods on the abstract query class.

[djmattyg007]

It's probably best to wait for #142 to be resolved before trying to make changes to this.

[harikt]

I didn't noticed it earlier. Thank you for the mention.