remove test_data/config_dirs, create it on the fly #1216
Conversation
xcheng-here
force-pushed
the
master
branch
2 times, most recently
from
83da02d to
0022607
Compare
Codecov Report
@@ Coverage Diff @@
## master #1216 +/- ##
==========================================
- Coverage 85.59% 79.36% -6.23%
==========================================
Files 231 168 -63
Lines 17392 10029 -7363
==========================================
- Hits 14886 7960 -6926
+ Misses 2506 2069 -437
Continue to review full report at Codecov.
|
| /* Parse multiple config files in a directory. */ | ||
| TEST(config, OneDir) { | ||
| Config config(std::vector<boost::filesystem::path>{"tests/test_data/config_dirs/one_dir"}); | ||
| TemporaryDirectory temp_dir; | ||
| std::vector<std::string> &&dirs = generate_multi_config(temp_dir); |
There was a problem hiding this comment.
I don't think we have a lot of assignments to r-value references like that in our code. Can you explain why it would help?
There was a problem hiding this comment.
No special reason. In my understanding, r-value reference could avoid an unnecessary copy and that function could return an unamed variant in stack. I believe it also works if I declare variant out side the generate_multi_config() function and pass it's reference into that function, basically the cost of those two ways are same.
| @@ -169,19 +169,35 @@ TEST(config, TomlConsistentNonempty) { | |||
| EXPECT_EQ(conf_str1, conf_str2); | |||
| } | |||
|
|
|||
| static std::vector<std::string> generate_multi_config(TemporaryDirectory &temp_dir) { | |||
| Utils::writeFile((temp_dir / "a_dir/a.toml"), std::string("[storage]\npath = \"path_a\"\n\n[pacman]\nos = \"os_a\"")); | |||
There was a problem hiding this comment.
For a bit more readability, it could use the same style as in the TwoTomlCorrectness test, where new lines are reflected in the source.
There was a problem hiding this comment.
OK, thanks, I'll modify it.
| std::string("[storage]\npath = \"latest_path\"\n\n[provision]\nprovision_path = \"y_prov_path\"")); | ||
| Utils::writeFile((temp_dir / "b_dir/z.txt"), std::string("[storage]\npath = \"this is path from text file\"")); | ||
|
|
||
| return std::vector<std::string>{(temp_dir / "a_dir").c_str(), (temp_dir / "b_dir").c_str()}; |
There was a problem hiding this comment.
boost::filesystem::path has a .string() method which converts into std::string. If you use c_str(), it would probably do two conversions needlessly.
|
It looks like the reason codecov is showing a big coverage decrease is that your branch is based on fef78cc, which is a few days old and did not include Mike's new test. If you rebase your branch on master, it should show better numbers. |
those data are used for unit test of config module, easy to create in unit test, not necessary put these files in git repositoryt push --force-with-lease origin master Signed-off-by: cheng.xiang <ext-cheng.xiang@here.com>
those data are used for unit test of config module,
easy to create in unit test, not necessary put these
files in git repository