Use unique name for AlertNotify tempfile #6527

pull casey wants to merge 1 commits into bitcoin:master from casey:alert-tests-tempfile changing 1 files +2 −2

casey commented at 3:11 AM on August 6, 2015: contributor

Fixes #6524

Use a unique name for alertnotify.txt tempfile during AlertNotify test
Use unique name for AlertNotify tempfile 231c5601a1
in src/test/alert_tests.cpp:None in 231c5601a1
```
 161 | @@ -162,8 +162,8 @@ BOOST_AUTO_TEST_CASE(AlertNotify)
 162 |      SetMockTime(11);
 163 |      const std::vector<unsigned char>& alertKey = Params(CBaseChainParams::MAIN).AlertKey();
 164 |  
 165 | -    boost::filesystem::path temp = GetTempPath() / "alertnotify.txt";
 166 | -    boost::filesystem::remove(temp);
 167 | +    boost::filesystem::path temp = GetTempPath() /
 168 | +        boost::filesystem::unique_path("alertnotify-%%%%.txt");
```
jonasschnelli commented at 6:46 AM on August 6, 2015:

minor nit: wrong indent (same on L171 / not related to this PR)

casey commented at 2:15 PM on August 6, 2015:

Isn't the right indentation four spaces? Or is it different for a line continuation?

jonasschnelli commented at 2:17 PM on August 6, 2015:

Meh. Sorry. I oversaw the line break... all good! You can always use clang-format -i <file> if you wan't to confirm to the code-style-guidelines.
randy-waterhouse commented at 6:47 AM on August 6, 2015: contributor

tested ACK
jonasschnelli commented at 6:48 AM on August 6, 2015: contributor

ACK. Couldn't find any details about when files generated with boost::filesystem::unique_path are deleted. I think adding a boost::filesystem::remove(temp); wouldn't harm?
laanwj commented at 9:45 AM on August 6, 2015: member

ACK. Using a hardcoded/predictable temporary file name can be dangerous.

And agree with @jonasschnelli that the test should try to clean up after itself.
casey commented at 2:16 PM on August 6, 2015: contributor

@jonasschnelli @laanwj It actually does clean up after itself; there's a call to boost::filesystem::remove(temp) at the end of the function.
jonasschnelli commented at 2:19 PM on August 6, 2015: contributor

@casey: Ah. Now i see how this works. The boost::filesystem::remove(temp); removed in this PR was there because the file was not really a random tmp file.

ACK.
laanwj commented at 6:21 AM on August 7, 2015: member

Oh! oops. Apologies for the spurious comment, then.
laanwj merged this on Aug 7, 2015
laanwj closed this on Aug 7, 2015
laanwj referenced this in commit 41d650ff92 on Aug 7, 2015
MarcoFalke locked this on Sep 8, 2021

Contributors

Linked (view graph)

#6524 make check leaves behind /tmp/alertnotify.txt