btc-ag / service-idl

Xtext-based Service IDL (Interface Definition Language) and Code Generators for Protobuf, C++, Java and .NET
Eclipse Public License 2.0
8 stars 8 forks source link

Add support for import keyword (CPP only) #227

Closed huttenlocher closed 5 years ago

huttenlocher commented 5 years ago

Fixes C++ part of #186

sigiesec commented 5 years ago

We need to add a test case to https://bitbucket.e-konzern.de/projects/BTCCABCOM/repos/serviceidl-integrationtests/browse to test this. The test driver in test.py will need to be extended to support this. Can you give this a look as well? If you need my support, don't hesitate to contact me.

sigiesec commented 5 years ago

Finally, the eclipse downloads worked again, and the serviceidl-integrationtests could run. As you can see, one test case (nested-module-in-non-empty-module) is currently failing. Please give this a look.

In addition, I thought a bit on how to extend the integration tests. In the YAML files, there already is an attribute, additional_input_files, to specify IDL files beyond the main file. This was thought to be used for imports, but as discussed, it did not work before. The command line constructed in the test_case method must be changed to use the import parameter, rather than specifying the files plainly. The attribute might be renamed to something referring to "import" explicitly. Import test cases could be implemented either by generating and building both the imported and the importing file within the serviceidl-integrationtests, or by only generating and building the importing file only, and using existing packages, for which we could add serviceidl-integrationtests- repositories. This would be easier to implement on the side of the serviceidl-integrationtests, but might be problematic, if changes require updating the serviceidl-integrationtests- packages. But maybe we can just start with that, until it proves to be problematic in practice.

huttenlocher commented 5 years ago

I could not get integration tests to be executed locally in order to investigate this and add new one as you requested above. Please get in touch tomorrow to take a look on my machine. Thanks.

sigiesec commented 5 years ago

@huttenlocher Ok, I will get in touch with you tomorrow!

sigiesec commented 5 years ago

I created #238 that combines this and the .NET PR, and resolved the conflicts, and squashed some commits together to get a cleaner commit history.