Feedback: Testing on windows

ThaDafinser commented 8 years ago

Hello,

like already talked on twitter, i did now a test for the extension on windows.

I've tested all classes/methods, expect (never needed them until yet)

Connection::setIniPath();
Connection::reloadIniFile();

Following things and ideas came to my mind while testing

Rename Connection::attributes();

To Connection::getAttributes() maybe? https://github.com/gkralik/php7-sapnwrfc/blob/master/docs/sapnwrfc.stubs.php#L71

Trace level off does not work as expected

When i use TRACE_LEVEL_OFF i thought no trace file will be created, but there was minimal one -> error or just a missinterpretion from me? https://github.com/gkralik/php7-sapnwrfc/blob/master/docs/sapnwrfc.stubs.php#L47

Exception

I could not manage to throw here the Exceptions https://github.com/gkralik/php7-sapnwrfc/blob/master/docs/sapnwrfc.stubs.php#L68 https://github.com/gkralik/php7-sapnwrfc/blob/master/docs/sapnwrfc.stubs.php#L78

errorInfo to private?

Since there is a public method to get the data (btw that's an awesome info!) https://github.com/gkralik/php7-sapnwrfc/blob/master/docs/sapnwrfc.stubs.php#L25

Two trace files

When executing this, you'll have two trace files

use SAPNWRFC\Connection as SapConnection;

SapConnection::setTraceLevel(SapConnection::TRACE_LEVEL_BRIEF)
SapConnection::setTraceDir('my-dir');

$config = [...];
$conn = new SapConnection($config);

SapConnection::setTraceDir('another-dir');
$func = $conn->getFunction('A_RFC_FUNCTION');

Strange trace duplication at the start

The call to RfcUTF8ToSAPUC seems to happen pretty often.


........................

<< RfcSetTraceDir returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcUTF8ToSAPUC
<< RfcUTF8ToSAPUC returned RFC_OK

>> RfcOpenConnection

.....................

gkralik commented 8 years ago

Thanks for testing :+1:

Rename Connection::attributes()

Sounds reasonable, getAttributes() would be fine and it would match getFunction(), etc.

Trace level off does not work as expected

When i use TRACE_LEVEL_OFF i thought no trace file will be created, but there was minimal one -> error or just a missinterpretion from me?

As far as I understood the comments in the NW RFC SDK headers, TRACE_LEVEL_OFF should really disable trace files. I'm going to check that again on my testing VM as I can't remember whether I'd observed similar or different behaviour here. But I don't think we really can do much if the underlying call doesn't do what it says in the docs...

Exception

I could not manage to throw here the Exceptions https://github.com/gkralik/php7-sapnwrfc/blob/master/docs/sapnwrfc.stubs.php#L68 https://github.com/gkralik/php7-sapnwrfc/blob/master/docs/sapnwrfc.stubs.php#L78

Those exceptions are thrown when the connection is closed or lost before executing the functions. See the tests for examples (tests/019_attributes_throws_exception_if_connection_closed.phpt and 022_ping_throws_exception_if_connection_closed.phpt).

errorInfo to private?

Seconded. I will change that.

Two trace files

That's expected. The trace file is opened as soon as the connection is opened, which happens in the constructor. Changing the trace directory or the trace level does not affect already written trace information.

Strange trace duplication at the start

The calls to RfcUTF8ToSAPUC are needed by the NW RFC SDK. All user input from the PHP side has to be converted. The reason for that happening a lot when opening the connection is that the key => value pairs of the connection parameters need to be converted, so that makes two calls per option given plus a few more.

gkralik commented 8 years ago

See #20 and #21

gkralik / php7-sapnwrfc