Skip to content

Add testing of synchronization script with virtual screen buffer #167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

vmdocua
Copy link
Collaborator

@vmdocua vmdocua commented Jun 16, 2025

  • Setup and test xvfb locally on dev machine and provide instructions how to run scripts.
  • Add background xdotool-based process generating keyboard events to simulate pulse series with 5 key and q or ESC at the end.
  • Configure GitHub test action for reprostim timesync-stimuli command with virtual screen buffer to test it automatically with automatically generated container (Add testing of reprostim psychopy container build #146).
  • Research if possible to do screen capture via xvfb-run or related tool like import or ffmpeg so we could test for qrcodes etc.
  • Upload screenshots and captured screen videos artifacts to GitHub.

Closes #147.

@vmdocua vmdocua self-assigned this Jun 16, 2025
Copy link

codecov bot commented Jun 17, 2025

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 4.44%. Comparing base (d9021bd) to head (2d567a0).
Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
src/reprostim/cli/cmd_timesync_stimuli.py 0.00% 1 Missing ⚠️
src/reprostim/qr/timesync_stimuli.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           master    #167      +/-   ##
=========================================
- Coverage    4.46%   4.44%   -0.03%     
=========================================
  Files          19      19              
  Lines        1924    1933       +9     
  Branches      262     263       +1     
=========================================
  Hits           86      86              
- Misses       1838    1847       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vmdocua vmdocua added documentation Changes only affect the documentation enhancement New feature or request patch Increment the patch version when merged release Create a release when this pr is merged labels Jun 17, 2025

- name: Test timesync-stimuli run
run: |
export FRAME_WIDTH=1920
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally place this into a script which could even be used locally .

Do not rely on /tmp - use ${TMPDIR:-/tmp}

Comment on lines +99 to +103





Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

Copy link
Member

@yarikoptic yarikoptic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments + would be great to actually do the analysis of the grabbed .avi -- do qr parsing and see if timing is congruent with the "design".

export FRAME_HEIGHT=1080
export FRAME_RATE=60
export FRAME_BPP=24
export DISPLAY_PATH="/tmp/reprostim_last_display.txt"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not hardcode paths under folders which are public, use mktemp , e.g. smth like $(mktemp -d ${TMPDIR:-/tmp}/dl-XXXXXXX) if you want an entire folder to place stuff in.

./test_reprostim_events.sh 2 5 5 1.5 20 "${DISPLAY}" &
echo "Record video for 45 seconds"
ffmpeg -video_size "${FRAME_WIDTH}x${FRAME_HEIGHT}" -framerate "${FRAME_RATE}" -f x11grab -i "$DISPLAY" -t 45 -c:v libx264 -pix_fmt yuv420p "/tmp/reprostim_screenshot_$(date +%Y-%m-%d_%H-%M-%S).mp4"
sleep 45
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may be move this entire block into REPROSTIM_CMD which would already have DISPLAY

# kill Xvfb at the end
sleep 1
kill $XVFB_RUN_PID
wait $XVFB_RUN_PID 2>/dev/null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when you move all those commands into a script, there would be no need for duplicating the commands here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Changes only affect the documentation enhancement New feature or request patch Increment the patch version when merged release Create a release when this pr is merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add testing of synchronization script
2 participants