qa: Fix race in feature_notifications #15364

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:Mf1902-qaNotifRace changing 1 files +3 −5

MarcoFalke commented at 5:26 PM on February 7, 2019: member

After creating the alertnotify file, the file might still be opened for writing on Windows, resulting in a race when it is deleted right after.

Solve that by not deleting the file at all.

Log on appveyor: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/22199287?fullLog=true#L51
qa: Fix race in feature_notifications fabcfbcb8c
MarcoFalke added the label Tests on Feb 7, 2019

in test/functional/feature_notifications.py:82 in fabcfbcb8c

  82 |          self.nodes[1].generatetoaddress(2, ADDRESS_BCRT1_UNSPENDABLE)
  83 |          self.sync_all()
  84 |  
  85 |          self.log.info("-alertnotify should not continue notifying for more unknown version blocks")
  86 | -        assert_equal(len(os.listdir(self.alertnotify_dir)), 0)
  87 | +        assert_equal(len(os.listdir(self.alertnotify_dir)), 1)

promag commented at 2:30 PM on February 8, 2019:

I suppose it could generate the same notification and then the file would be the same — there could be a race to the same file though — but maybe we should ensure the alert notify command always creates a new file, for instance, append the timestamp to the filename?

promag commented at 2:30 PM on February 8, 2019: member

utACK fabcfbc.

nit, update year 😛
MarcoFalke commented at 3:53 PM on February 8, 2019: member

Closing. Probably not worth changing the test for that.
MarcoFalke closed this on Feb 8, 2019
MarcoFalke deleted the branch on Feb 8, 2019
DrahtBot locked this on Dec 16, 2021

Contributors

MarcoFalke

promag

Labels

Tests