Skip to content
This repository has been archived by the owner on Oct 4, 2019. It is now read-only.

Research and analyze fc::reflect work. Get reflection reference values. Write tests. #466

Open
AKorpusenko opened this issue Mar 14, 2018 · 9 comments

Comments

@AKorpusenko
Copy link

  1. Analyze code of fc::reflect. Figure out what exactly fc::reflect is able to handle.
  2. Write a script which analyzes the golos code base and returns all usages FC_REFLECT (and etc) macro to get the reference values. It can be presented as a simple list.
  3. Cover all cases of fc::reflect usage with tests.
@AKorpusenko AKorpusenko added WIP work in progress (4 wip bot) core R&D low labels Mar 14, 2018
@AKorpusenko AKorpusenko self-assigned this Mar 14, 2018
@AKorpusenko
Copy link
Author

This issue is pretty big, so we've decided to split the task on few basic steps.

  1. Write a test for a random structure which was reflected with FC_REFLECT. Fill the members of that structure with some values and check that fc::raw::pack work ok with it. Check that unpack is still working ok. Check that fc::variant works ok with this structure too, I also mean to_string and from_string methods.
  2. Write a code generator script which generates such checks for all reflected structures in the project (and greps them first). Write separate functions for different reflect macroses.

@AKorpusenko AKorpusenko added this to In progress in R&D Mar 21, 2018
AKorpusenko added a commit that referenced this issue Mar 21, 2018
…. Added md document with scripts work result. #466
AKorpusenko added a commit that referenced this issue Mar 21, 2018
@AKorpusenko
Copy link
Author

I've decided to write full description of script work.

  • Script has to go through all .hpp, .cpp, .h files in source code and find all calls of FC_REFLECT, and after that generate some information for every structure.

  • This information should contain next:

  1. struct / class name like "golos::core::some_class"
  2. fields with their types like fields["obj_id"] -> uint64_t
  3. path to the file where structure is declared like golos/plugins/some_plugin/plugin.cpp
  4. The name of the structure from which the structure is inherited (If FC_REFLECT_DERIVED was called)
  5. list of all fields (current + inherited) like in point 2
  • After reading point 5 you may make a correct guess that the one of the primary aims of scripts work is to generate dependency graph for correct c++ tests generation

  • There is a huge trouble with different usage of reflect macroses. Especially painful are cases when macro is called from file which different with file where struct was declared 🔥 🔥 🔥 .

@AKorpusenko
Copy link
Author

Right now 128 of 275 structures have been covered with randomized tests. @kotbegemot , @marijadia

@kotbegemot kotbegemot added this to In progress in development Mar 30, 2018
@AKorpusenko
Copy link
Author

AKorpusenko commented Apr 2, 2018

@marijadia , @kotbegemot
So, task status for today:

  1. All reflected structures are covered with the tests. You can check this out in branch 466 and last commits.
  2. Test are commented (with //) only for enums and database objects (which inherit chainbase object and got no default constructor)
  3. There are some problems with compiling. Solving them.

@AKorpusenko
Copy link
Author

The main trouble now is that there are tons of structures that can not be packed with fc::pack.

AKorpusenko added a commit that referenced this issue Apr 3, 2018
AKorpusenko added a commit that referenced this issue Apr 4, 2018
AKorpusenko added a commit that referenced this issue Apr 5, 2018
…s. Started figuring out how to fix database objects tests. #466
@AKorpusenko
Copy link
Author

AKorpusenko commented Apr 5, 2018

🛠️ As u can see in latest commit. I've made possible to create database objects inside of a test case. For example:

           db = & appbase::app().get_plugin<golos::plugins::chain::plugin>().db();
           const auto& v1 = db->create<golos::plugins::private_message::message_object>(
               [&]( golos::plugins::private_message::message_object& obj ) {
               set_random_value( obj.from );
               set_random_value( obj.to );
               set_random_value( obj.from_memo_key );
               set_random_value( obj.to_memo_key );
               set_random_value( obj.sent_time );
               set_random_value( obj.receive_time );
               set_random_value( obj.checksum );
               set_random_value( obj.encrypted_message );

           } );

           auto& v2 = db->create<golos::plugins::private_message::message_object>(
               [&]( golos::plugins::private_message::message_object& obj ) {
           } );

🤕 The main trouble for now is unpacking database objects (using fc::raw::unpack).

@AKorpusenko
Copy link
Author

First, #518 have to be done. ⚙️

AKorpusenko added a commit that referenced this issue Apr 13, 2018
@AKorpusenko
Copy link
Author

AKorpusenko commented Apr 16, 2018

Issue #518 is closed as there is no need to do what's written there to move forward.

  • So, the first subtask for now is to mark what reflected structures are inherited from chainbase::object and add variability of behavior to tests generator (initialization way of database objects and regular objects differ, but now generator doesn't know anything about that 🔨)
  • The second subtask is to make every test write result in a separated file. Every file file should be placed in special way:
    For instance golos::lib::code::abacaba -> reflect/results/golos/lib/code/abacaba
    That would be very useful for validate correctness of reflect work for every structure one by one.
  • Added exact seeder for every structure to determine random result. For example get a checksum of structure full name

@AKorpusenko
Copy link
Author

AKorpusenko commented Apr 16, 2018

@kotbegemot , @afalaleev
I've got problem with fc::raw::pack when it tries to pack any object which uses boost::interprocess::allocator. For now I comment down all such cases to make tests compile. But in some test cases such errors can not be resolved at all, because fc::raw::pack wasn't made in such way to cover this cases.

For example u can look at golos::chain::category_object and it's field shared_string name;. As you can see shared_string is a typedef: typedef boost::interprocess::basic_string<char, std::char_traits<char>, allocator<char>> shared_string;

@nemothenoone nemothenoone removed the epic label Apr 25, 2018
@nemothenoone nemothenoone removed the epic label Apr 26, 2018
@afalaleev afalaleev moved this from In progress to To do in development May 3, 2018
@zxcat zxcat removed the WIP work in progress (4 wip bot) label Jul 18, 2018
@afalaleev afalaleev removed the core label Feb 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
development
  
To do
R&D
  
In progress
Development

No branches or pull requests

4 participants