Skip to content

[Bugfix:InstructorUI] Fix DockerUI instability#12216

Merged
bmcutler merged 11 commits intomainfrom
test-dockerui
Nov 25, 2025
Merged

[Bugfix:InstructorUI] Fix DockerUI instability#12216
bmcutler merged 11 commits intomainfrom
test-dockerui

Conversation

@williamschen23
Copy link
Contributor

@williamschen23 williamschen23 commented Nov 17, 2025

Why is this Change Important & Necessary?

Docker UI has been failing. The reason is that the tests run too fast. Look at the following snippet to see reason:

root@vagrant:/usr/local/submitty/config# stat autograding_containers.json 
  File: autograding_containers.json
  Size: 681       	Blocks: 8          IO Block: 4096   regular file
Device: fd00h/64768d	Inode: 2274614     Links: 1
Access: (0660/-rw-rw----)  Uid: ( 1001/submitty_php)   Gid: ( 1001/submitty_daemonphp)
Access: 2025-11-23 22:22:11.833514610 -0500
Modify: 2025-11-23 22:22:11.781514464 -0500
Change: 2025-11-23 22:22:11.781514464 -0500
 Birth: 2025-11-17 18:44:18.299287221 -0500
root@vagrant:/usr/local/submitty/config# fil^C
root@vagrant:/usr/local/submitty/config# stat /var/local/submitty/logs/docker/20251123.txt 
  File: /var/local/submitty/logs/docker/20251123.txt
  Size: 95008     	Blocks: 200        IO Block: 4096   regular file
Device: fd00h/64768d	Inode: 736155      Links: 1
Access: (0755/-rwxr-xr-x)  Uid: ( 1003/submitty_daemon)   Gid: ( 1001/submitty_daemonphp)
Access: 2025-11-23 22:22:11.513513716 -0500
Modify: 2025-11-23 22:22:11.029512364 -0500
Change: 2025-11-23 22:22:11.029512364 -0500
 Birth: 2025-11-23 22:12:50.427670218 -0500
root@vagrant:/usr/local/submitty/config#

Modify: 2025-11-23 22:22:11.781514464 -0500
Modify: 2025-11-23 22:22:11.029512364 -0500

With nanosecond precision, we can see this error clearly. However, PHP's filemtime stops at the seconds level, as shown by the code below

                $containers_mtime = filemtime($containers_config_path);
                $last_update_mtime = filemtime($last_update_logpath);

                echo "Containers mtime: " . $containers_mtime . "\n";
                echo "Last update mtime: " . $last_update_mtime . "\n";
                // If the containers config is older than or the same age as the last log, it's up to date.
                if ($containers_mtime <= $last_update_mtime) {
                    $this->update_needed = false;
                }

Containers mtime: 1763954531
Last update mtime: 1763954531

What is the New Behavior?

The most simple fix would be to change the docker test to introduce a one second delay. However, this would only fix the test and not the production grade issue that it can cause. However, since PHP does not have a built in function to get file time by the nanoseconds (if the operating system has nanoseconds in the first place), this would be pretty hard to implement. Furthermore, this should rarely pop up as instructors are usually not updating docker / remove docker / adding docker images in the same nanosecond. Would be open to feedback, but as of now, I will add a one second wait to the docker test to fix this issue.

What steps should a reviewer take to reproduce or test the bug or new feature?

Look above

Automated Testing & Documentation

Other information

@github-project-automation github-project-automation bot moved this to Seeking Reviewer in Submitty Development Nov 17, 2025
@automateprojectmangement automateprojectmangement bot moved this from Seeking Reviewer to Work in Progress in Submitty Development Nov 17, 2025
@williamschen23 williamschen23 changed the title test dockerui cahnges test dockerui changes Nov 17, 2025
@codecov
Copy link

codecov bot commented Nov 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 21.71%. Comparing base (d7f9077) to head (da35ff8).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main   #12216   +/-   ##
=========================================
  Coverage     21.71%   21.71%           
  Complexity     9612     9612           
=========================================
  Files           268      268           
  Lines         36070    36070           
  Branches        475      475           
=========================================
  Hits           7832     7832           
  Misses        27767    27767           
  Partials        471      471           
Flag Coverage Δ
autograder 21.39% <ø> (ø)
js 2.07% <ø> (ø)
migrator 100.00% <ø> (ø)
php 20.71% <ø> (ø)
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs 90.72% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@williamschen23 williamschen23 marked this pull request as ready for review November 24, 2025 04:18
@automateprojectmangement automateprojectmangement bot moved this from Work in Progress to Seeking Reviewer in Submitty Development Nov 24, 2025
@automateprojectmangement automateprojectmangement bot moved this from Seeking Reviewer to In Review in Submitty Development Nov 24, 2025
@williamschen23 williamschen23 changed the title test dockerui changes [Bugfix:InstructorUI] Fix DockerUI instability Nov 24, 2025
Copy link
Contributor

@Chriun Chriun left a comment

Choose a reason for hiding this comment

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

Didn't test, but the debugging done here definitely seems correct. I agree that since it wouldn't be likely that this would occur in production we can just add an explicit wait, but it would be good to keep in mind if something does happen in the future.

@github-project-automation github-project-automation bot moved this from In Review to Awaiting Maintainer Review in Submitty Development Nov 24, 2025
@bmcutler bmcutler merged commit 4512af4 into main Nov 25, 2025
50 of 51 checks passed
@bmcutler bmcutler deleted the test-dockerui branch November 25, 2025 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants