nhsuk / ui-test-core

Python package which helps with writing UI tests by providing a wrapper around Selenium and other useful functions
MIT License
12 stars 6 forks source link

[BUG] Concurrency issue with take_screenshot and move_screenshots_to_folder #49

Open JonathanJackson14 opened 3 months ago

JonathanJackson14 commented 3 months ago

Describe the bug The method BrowserHandler.move_screenshots_to_folder will currently move any file of type ".png" into the target folder. This means that, if you have many threads concurrently saving .png files into the screenshots directory, there is a possibility that one thread may move screenshots created by the other thread into the wrong target folder.

To Reproduce (Note: due to the nature of this issue, it is hard to reproduce.) Steps to reproduce the behavior:

  1. In one thread, call take_screenshot to save a .png file to the /screenshots folder.
  2. In another parallel thread, call take_screenshot again to save another .png file to the /screenshots folder.
  3. In the first thread, now call move_screenshots_to_folder to move the .png file to the given target folder. Both the .png files created in steps 1 and 2 will be moved to the target folder, not just the screenshot taken by this thread.
  4. In the second thread, now also call move_screenshots_to_folder. There will be no screenshots to move.

Expected behavior Without breaking the existing behaviour, we need to provide:

Additional context This problem arose for our team because we run our acceptance tests parallelised per feature file in concurrently running docker containers, but volume mounted with a shared /screenshots directory on the running agent. We initially solved this problem by adding a fork of these two static methods in our own repo, but we want to include a bug fix for the underlying issue in the library so that we don't have to keep maintaining a duplicate of this functionality.