#175 fixed #242

yury-trofimov · 2018-11-02T12:54:42Z

please to review

sigiesec

At least one test case must be added that uses this functionality, to the files in com.btc.serviceidl.test/.../good, and this must also be included in the serviceidl-integrationtests.

YT: Right, we need some tests. That should be done within the ticket PRINS-4407 in the one of the next sprints. @huttenlocher extended the ticket.

I am not quite sure how the ODB project generation can be controlled, i.e. how I would run a generation on something that has an ID field, but not ODB project should be generated. Maybe the pre-defined project sets need to be updated?

YT: @GerrietReents suggested to use -projectSet parameter (with odb value) to specify the necessity of using ODB. However it requires to support of multivalued parameter, something like api|server|odb. New ticket should be open for this purpose.

...erviceidl/src/com/btc/serviceidl/generator/cpp/cmake/CMakeTopLevelProjectFileGenerator.xtend

com.btc.serviceidl/src/com/btc/serviceidl/generator/cpp/prins/OdbProjectGenerator.xtend

com.btc.serviceidl/src/com/btc/serviceidl/generator/cpp/TypeResolver.xtend

com.btc.serviceidl/src/com/btc/serviceidl/generator/DefaultGenerationSettingsProvider.xtend

...btc.serviceidl/src/com/btc/serviceidl/generator/cpp/cmake/CMakeModuleStructureStrategy.xtend

sigiesec · 2018-11-12T09:32:18Z

There are a few unit tests failing: https://travis-ci.org/btc-ag/service-idl/builds/452915114#L7625
In addition, --most-- some integration test cases are failing: https://bpaste.net/show/457d7b9a18a1
As already mentioned above, the integration tests need to be extended to make use of the new feature.
As already mentioned above, it must be possible to control the generation of an ODB project in some way. The default should be the behaviour prior to this PR, so that existing users are not affected.

The PR cannot be merged before all of these points have been solved.

sigiesec · 2018-11-12T09:51:43Z

Maybe the failing integration tests are due to inconsistent conan packages, since there have been several build failures on the CAB jenkins over the last days due to http://jira.e-konzern.de/browse/BOPINFRA-582

huttenlocher · 2018-11-12T10:42:37Z

Maybe the failing integration tests are due to inconsistent conan packages, since there have been several build failures on the CAB jenkins over the last days due to http://jira.e-konzern.de/browse/BOPINFRA-582

Probably yes, just now I checked out the master branch and tried to run the integration tests, same tests are failing also with "master". There is probably something wrong in the "lastest" package dependencies. I get linker errors in almost all projects for elementary classes like BTC::ServiceComm::API::InvalidReplyReceivedyException etc. which are correctly referenced in CMake files and not touched by last commits.

huttenlocher · 2018-11-12T10:51:23Z

As already mentioned above, the integration tests need to be extended to make use of the new feature.

Yes, we keep one eye on this, however we will not be able provide an integration test in this sprint. Instead, we will provide new integration tests along with new tests for #186; this is planned as Jira issue PRINS-4407. We need also to check whether all conditions are fulfilled, at least to clarify: does @yury-trofimov has permissions to commit to Bitbucket repository bitbucket.e-konzern.de/scm/btccabcom/serviceidl-integrationtests.git

com.btc.serviceidl/src/com/btc/serviceidl/generator/cpp/prins/OdbProjectGenerator.xtend

sigiesec · 2018-11-14T11:11:27Z

The serviceidl-integrationtests now work again in general, however some test cases are still failing.
This includes test cases that currently activate the ODB behaviour (which must be changed anyway as indicated above), apparently because they generate the PRINS encapsulation headers:

[ERROR<] Error when processing test case 'cpp-current', module 'None': 
[ERROR-] conan build (conan build ..) failed with exit code 1 and output: Project: Running build()
[ERROR-] D:\NLS\serviceidl-integrationtests\output-78\cpp\Infrastructure\ServiceHost\Demo\API\ExternalDBImpl\odb\entrytype.hxx:4:10: fatal error: modules/Commons/include/BeginPrinsModulesInclude.h: No such file or directory
[ERROR-]  #include "modules/Commons/include/BeginPrinsModulesInclude.h"
[ERROR-]           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[ERROR-] compilation terminated.
[ERROR-] 
[ERROR-] ----Running------
[ERROR-] > bin\protoc.exe --proto_path=D:\NLS\serviceidl-integrationtests\output-78\cpp --cpp_out="D:\NLS\serviceidl-integrationtests\output-78\cpp" D:\NLS\serviceidl-integrationtests\output-78\cpp\Infrastructure\ServiceHost\Demo\API\Protobuf\gen\DemoX.proto D:\NLS\serviceidl-integrationtests\output-78\cpp\Infrastructure\ServiceHost\Demo\API\Protobuf\gen\Types.proto
[ERROR-] -----------------
[ERROR-] 
[ERROR-] ----Running------
[ERROR-] > C:\.conan\r4c91d7j\1\bin\odb.exe --std c++14 -I C:\.conan\j6uv53n7\1\include -I C:\.conan\r4c91d7j\1\include -I C:\.conan\7b1t311t\1\include --multi-database dynamic --database common --database mssql --database oracle --generate-query --generate-prepared --generate-schema --schema-format embedded -x -Wno-unknown-pragmas -x -Wno-pragmas -x -Wno-literal-suffix -x -Wno-attributes --hxx-prologue "#include "traits.hxx"" --output-dir D:\NLS\serviceidl-integrationtests\output-78\cpp\Infrastructure\ServiceHost\Demo\API\ExternalDBImpl\odb D:\NLS\serviceidl-integrationtests\output-78\cpp\Infrastructure\ServiceHost\Demo\API\ExternalDBImpl\odb\entrytype.hxx
[ERROR-] -----------------
[ERROR-] ERROR: BTC.PRINS.Infrastructure.ServiceHost.Demo.API/0.1.0-unreleased-build.local@PROJECT: Error in build() method, line 52
[ERROR-] 	self.generateODBFiles()
[ERROR-] while calling 'generateODBFiles', line 41
[ERROR-] 	' --multi-database dynamic --database common --database mssql --database oracle --generate-query --generate-prepared --generate-schema --schema-format embedded -x -Wno-unknown-pragmas -x -Wno-pragmas -x -Wno-literal-suffix -x -Wno-attributes --hxx-prologue "#include \"traits.hxx\"" --output-dir ' + odbdir + ' ' + odbdir + '\\entrytype.hxx')
[ERROR-] 	ConanException: Error 1 while executing C:\.conan\r4c91d7j\1\bin\odb.exe --std c++14 -I C:\.conan\j6uv53n7\1\include -I C:\.conan\r4c91d7j\1\include -I C:\.conan\7b1t311t\1\include --multi-database dynamic --database common --database mssql --database oracle --generate-query --generate-prepared --generate-schema --schema-format embedded -x -Wno-unknown-pragmas -x -Wno-pragmas -x -Wno-literal-suffix -x -Wno-attributes --hxx-prologue "#include "traits.hxx"" --output-dir D:\NLS\serviceidl-integrationtests\output-78\cpp\Infrastructure\ServiceHost\Demo\API\ExternalDBImpl\odb D:\NLS\serviceidl-integrationtests\output-78\cpp\Infrastructure\ServiceHost\Demo\API\ExternalDBImpl\odb\entrytype.hxx
[ERROR-] 
[ERROR>]

The test case interface-query-raises-exception is also failing with a different problem:

[ERROR-] CMake Error at CMakeLists.txt:24 (include):
[ERROR-]   include could not find load file:
[ERROR-] 
[ERROR-]     D:/NLS/serviceidl-integrationtests/output-79/cpp/foo/ExternalDBImpl/build/make.cmakeset

huttenlocher · 2018-11-14T11:14:45Z

The serviceidl-integrationtests now work again in general, however some test cases are still failing.

Nice, we can then investigate those problems locally until everything is working. We will tackle this as soon as possible today.

huttenlocher · 2018-11-23T08:04:07Z

Now all integration tests are green. To address conditional ODB generation, I created the new issue #244, since this functionality goes beyond the scope of #175 (= "make ODB compatible with CMake" - this basic requirement is now implemented). To address extension of integration tests, we already created the issue PRINS-4407. So for our part, we consider this ticket as solved.

sigiesec · 2018-11-23T09:20:44Z

I am sorry, but it is not possible to merge this, since it changes the behaviour for existing uses. This is an implicit constraint of the compatibility of versions. In addition, integration tests for the new functionality are still missing, as mentioned above.

sigiesec · 2018-11-23T09:23:25Z

We could merge this PR, if you change the behaviour to disable ODB generation by default now, and address conditional activation (and integration tests) in a later PR.

…ault

huttenlocher · 2018-11-23T12:00:12Z

Done. By default, ODB-based projects will not be generated (so previous behavior is still guaranteed). I introduced a new command line option value for OPTION_PROJECT_SET to force ODB generation, if desired.

sigiesec · 2018-11-23T16:32:27Z

Thanks for restoring the default behaviour.

I rebased the branch and squashed some commits to conform with the contribution guidelines , and I will open a new PR for that branch on your behalf, and close this one.

However, the addition of integration tests must still be done in a timely manner, also for the changes of #238. Please provide these by December 14th, otherwise I consider reverting the changes in the master again, until the integration test extensions are provided. This is essential to avoid maintain the quality of the Service IDL generators. On the one hand, I cannot judge if your additions work without such tests, but what is even more important is that no one can judge with reasonable effort if future changes break these new features.

sigiesec · 2018-11-23T17:53:03Z

See #246

PRINS-4000 fixed

fa753bf

yury-trofimov requested a review from a team November 2, 2018 12:54

yury-trofimov added 2 commits November 2, 2018 14:48

#175 fixed

8a38eba

change C++ standard for ODB compiler

ab963ca

sigiesec requested changes Nov 5, 2018

View reviewed changes

huttenlocher reviewed Nov 7, 2018

View reviewed changes

...btc.serviceidl/src/com/btc/serviceidl/generator/cpp/cmake/CMakeModuleStructureStrategy.xtend Outdated Show resolved Hide resolved

huttenlocher reviewed Nov 7, 2018

View reviewed changes

...btc.serviceidl/src/com/btc/serviceidl/generator/cpp/cmake/CMakeModuleStructureStrategy.xtend Outdated Show resolved Hide resolved

fixed some review points

b53efd5

Fixed failing unit tests

f876ea4

get rid of dependency on strategy

4e3af18

huttenlocher reviewed Nov 14, 2018

View reviewed changes

com.btc.serviceidl/src/com/btc/serviceidl/generator/cpp/prins/OdbProjectGenerator.xtend Outdated Show resolved Hide resolved

sigiesec reviewed Nov 14, 2018

View reviewed changes

com.btc.serviceidl/src/com/btc/serviceidl/generator/cpp/prins/OdbProjectGenerator.xtend Outdated Show resolved Hide resolved

Fixed invalid encoding of guillemets

6ffaef3

yury-trofimov and others added 3 commits November 14, 2018 18:03

naming fixed

e4c4865

fix for integration tests

6178638

Fixed failing integration tests

21d4ab5

Added conditional ODB generation by command line argument; off by def…

19434fd

…ault

sigiesec closed this Nov 23, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#175 fixed #242

#175 fixed #242

yury-trofimov commented Nov 2, 2018

sigiesec left a comment •

edited by yury-trofimov

Loading

sigiesec commented Nov 12, 2018 •

edited by huttenlocher

Loading

sigiesec commented Nov 12, 2018

huttenlocher commented Nov 12, 2018

huttenlocher commented Nov 12, 2018 •

edited

Loading

sigiesec commented Nov 14, 2018

huttenlocher commented Nov 14, 2018

huttenlocher commented Nov 23, 2018

sigiesec commented Nov 23, 2018

sigiesec commented Nov 23, 2018

huttenlocher commented Nov 23, 2018

sigiesec commented Nov 23, 2018

sigiesec commented Nov 23, 2018

#175 fixed #242

#175 fixed #242

Conversation

yury-trofimov commented Nov 2, 2018

sigiesec left a comment • edited by yury-trofimov Loading

Choose a reason for hiding this comment

sigiesec commented Nov 12, 2018 • edited by huttenlocher Loading

sigiesec commented Nov 12, 2018

huttenlocher commented Nov 12, 2018

huttenlocher commented Nov 12, 2018 • edited Loading

sigiesec commented Nov 14, 2018

huttenlocher commented Nov 14, 2018

huttenlocher commented Nov 23, 2018

sigiesec commented Nov 23, 2018

sigiesec commented Nov 23, 2018

huttenlocher commented Nov 23, 2018

sigiesec commented Nov 23, 2018

sigiesec commented Nov 23, 2018

sigiesec left a comment •

edited by yury-trofimov

Loading

sigiesec commented Nov 12, 2018 •

edited by huttenlocher

Loading

huttenlocher commented Nov 12, 2018 •

edited

Loading