Add settings merge test to prevent regresssions #15869

1. 
   ryanofsky commented at 5:32 pm on April 22, 2019: member

   Test-only change. Motivation: I’m trying to clean up settings code and add support for read/write settings without changing existing behavior, but current tests are very scattershot and don’t actually cover a lot of current behavior.
2. Add settings merge test to prevent regresssions 151f3e9cf1
3. DrahtBot added the label Tests on Apr 22, 2019
4. 
   jamesob commented at 0:33 am on April 27, 2019: member

   Concept ACK. Going to go line-by-line in the next few days.
5. 
   jonasschnelli commented at 7:21 am on April 29, 2019: contributor

   Thanks for doing this. utACK 151f3e9cf1bbcf30a4fc7749682e66b4a73ddfc2. I haven’t verified the sha256 check hash.
6. 
   practicalswift commented at 7:25 am on April 29, 2019: contributor

   Concept ACK

   Very nice! Regression testing is our best friend!

7. in src/test/util_tests.cpp:636 in 151f3e9cf1

   631+
   632+    //! Enumerate interesting combinations of actions.
   633+    template <typename Fn>
   634+    void ForEachActionList(Fn&& fn)
   635+    {
   636+        ActionList actions = {SET};
   

   ---

   ---

   

   MarcoFalke commented at 4:06 pm on April 29, 2019:

   I’d prefer if this didn’t set the first element to SET and the others to 0. Could do the following and add a comment that 0==SET? Also, on first sight this might look a bit like the list only has one element.

   0        ActionList actions = {};
   

8. MarcoFalke approved
9. 
   MarcoFalke commented at 4:39 pm on April 29, 2019: member

   utACK 151f3e9cf1bbcf30a4fc7749682e66b4a73ddfc2

   Signature:

    0-----BEGIN PGP SIGNED MESSAGE-----
    1Hash: SHA512
    2
    3utACK 151f3e9cf1bbcf30a4fc7749682e66b4a73ddfc2
    4-----BEGIN PGP SIGNATURE-----
    5
    6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
    7pUjGCwv/W8QAcQ1+K+tGRBF2LDzuHibOOtOOgsVh2E5yX953lVKmEZzhthhVLDW+
    8S3JlOYfc6l1q0gD1cIcX5IZuTk+pA+cMZIg2Co2xMHCoIk+UIrGe1UdxOwPX3AXZ
    9R3xmcErQHTZwD7oRQ4vZpt7BhA4ZPUxIh00CJ12z49KFm/Eh0C7zodalENuwwIvN
   107Rwd/kHXqA36w9JAF6JlxKmrLpftf+adIcEYjPyUldDnm+xF4M/VYxjR9XVjNkG4
   118bziYMA/kB8/g/30qHUzxfH5gXb1cptpbbA/CFXFLLaxYGx9lgui+sz8CNF/AzRQ
   12mgOF6vJJhrzm45Bz9Oo/uitgh/UGK3jDzImADcIWX9lE7WRQp0HAVH1AlwNm8gtY
   13ceMTlwj1gtAz2Y9Hz3kf8uUmHpkU+fqE4NJBO1J9n8wb0poW4DL/mnAo5N0UMdf6
   14ZY5iE+Y/uNLdYOzyO/XjmmKk6G5PP1/xNOz4xNthe26c34QkznpLHiCbRNRO4rs6
   15w7ihnATE
   16=SUNq
   17-----END PGP SIGNATURE-----
   

   Timestamp of file with hash c7ac440a2577d6ddd333508953cc427cd050c82938897165390cde198e35cbbc -

10. in src/test/util_tests.cpp:697 in 151f3e9cf1

    692+{
    693+    CHash256 out_sha;
    694+    FILE* out_file = nullptr;
    695+    if (const char* out_path = getenv("SETTINGS_MERGE_TEST_OUT")) {
    696+        out_file = fsbridge::fopen(out_path, "w");
    697+        if (!out_file) throw std::system_error(errno, std::generic_category(), "fopen failed");
    

    ---

    ---

    

    promag commented at 9:44 pm on April 29, 2019:

    BOOST_REQUIRE instead of throwing?
11. in src/test/util_tests.cpp:694 in 151f3e9cf1

    689+// compared against an expected hash. To debug, the result strings can be dumped
    690+// to a file (see below).
    691+BOOST_FIXTURE_TEST_CASE(util_SettingsMerge, SettingsMergeTestingSetup)
    692+{
    693+    CHash256 out_sha;
    694+    FILE* out_file = nullptr;
    

    ---

    ---

    

    promag commented at 9:45 pm on April 29, 2019:

    nit, FILE* out_file{nullptr};
12. in src/test/util_tests.cpp:608 in 151f3e9cf1

    603+struct SettingsMergeTestingSetup : public BasicTestingSetup {
    604+    //! Max number of actions to sequence together. Can decrease this when
    605+    //! debugging to make test results easier to understand.
    606+    static constexpr int MAX_ACTIONS = 3;
    607+
    608+    enum Action { SET = 0, NEGATE, SECTION_SET, SECTION_NEGATE, END };
    

    ---

    ---

    

    promag commented at 9:48 pm on April 29, 2019:

    Why = 0 since it’s already the default?
13. 
    promag commented at 9:49 pm on April 29, 2019: member

    Concept ACK.
14. 
    MarcoFalke commented at 4:14 pm on April 30, 2019: member

    Going to merge this, I think our style-nits can be fixed up later or not at all
15. MarcoFalke merged this on Apr 30, 2019
16. MarcoFalke closed this on Apr 30, 2019

    ---

17. MarcoFalke referenced this in commit 10ed4dff24 on Apr 30, 2019
18. ryanofsky referenced this in commit 87c923c7ca on May 3, 2019
19. ryanofsky referenced this in commit 43d6998991 on May 7, 2019
20. ryanofsky referenced this in commit 83a4467441 on May 8, 2019
21. ryanofsky referenced this in commit 05bfee3451 on May 9, 2019
22. sidhujag referenced this in commit e3fa75447c on May 15, 2019
23. HashUnlimited referenced this in commit 5dc01c6ff9 on Aug 30, 2019
24. laanwj referenced this in commit a7aec7ad97 on Nov 8, 2019
25. sidhujag referenced this in commit 16a52029e7 on Nov 9, 2019
26. deadalnix referenced this in commit eefe53fcf2 on Apr 27, 2020
27. sidhujag referenced this in commit c1677b430c on Nov 10, 2020
28. PastaPastaPasta referenced this in commit bba29576ce on Sep 11, 2021
29. Munkybooty referenced this in commit cf2c167c35 on Oct 12, 2021
30. DrahtBot locked this on Dec 16, 2021

Contributors
ryanofsky jamesob jonasschnelli practicalswift MarcoFalke promag

Labels
Tests