Move ocpp.FormationViolation to Endpoint (BC)

[andig]

Fix #227

TODO

• fix tests

[andig]

@lorenzodonini if you want to follow with this PR I would need your help with the tests. The client and centralsystem implementations are private, hence tests from different packages cannot access the endpoint afaikt. It could be an option to move these specific tests to the 1.6/2.0 folders them selves. Adding a public api seems too much.

The ocppj tests look wrong to me- since they never setup a server/client they cannot validate against endpoint's format.

[lorenzodonini]

The ocppj tests look wrong to me- since they never setup a server/client they cannot validate against endpoint's format.

What do you mean? Server/client are created for each test within the SetupTest function. Only the websocket layer is mocked.

What tests are broken exactly? Happy to help and think of something.

[andig]

What do you mean? Server/client are created for each test within the SetupTest function. Only the websocket layer is mocked.

Afaikt they create mock client/servers. Missing the endpoint they cannot be used to test for the endpoint's format.

[lorenzodonini]

All the failing tests are checking against the removed global var. To fix them, I suggest simply changing ocppj.FormationViolation into respectively:

• suite.chargePoint.Endpoint.FormatError or
• suite.centralSystem.Endpoint.FormatError

[andig]

Thank you, updated accordingly. That fixes it for ocppj. for ocpp16 and 20 the constructors are private and the tests are in a separate folder. Any idea how to tackle this?

[dwibudut]

Hello,
I upvote for this changes. With this change will not overwrite the ocppj.FormationViolation global value with the last one called (when start server each version). And I think for the fix test error need to split ocppj test for each version too (1.6 and 2.0.1) or add default ocppj_test version of FormatError for test only.

andig#1

[andig]

Tests currently panic since I have tried removing the default version for the test suite:

// defaultDialect := ocpp.V16 // set default to version 1.6 format error *for test only
// suite.centralSystem.Dialect = defaultDialect
// suite.chargePoint.Dialect = defaultDialect

Tbo, it is not clear for me why there should be a default version. Which default version should an endpoint have when it is being constructed? This feels like some sort of logical gap that we might fix while we're at it.

@lorenzodonini Is this a logical error in where the endpoint dialect (which is what I'm currently calling it) is being set or is this rather a mistake in the tests that should not test that specific property in these tests?

[dwibudut]

Error caused by dialect that not set to any version, and then the dialect value is 0, that it's made panic on ocppj.FormatErrorForDialect.

Suppose when ocppj.FormatErrorForDialect set default to returned empty string, this will error too in ErrorCode value validator.

Have you like to traced it with go test for first error method test, like:

go test -v ./ocppj -run ^TestMockOcppJ$ -testify.m TestCentralSystemInvalidMessageHook

[dwibudut]

To avoid mistake or logical gap, prefer need to make two different test version between:

defaultDialect := ocpp.V16

and

defaultDialect := ocpp.V2

This might be fairly test.

[andig]

Finished from my side. Didn‘t get rid of the test default but that should be acceptable.

[andig]

@lorenzodonini addressed comments- done. I like it, much appreciated!

Small comment: note the calls to suite.Equal- there is some potential to simplify the test code a bit.