Issue #113: open dbeaver app up and focus on it. Also fix behat/phpunit execution with public folder for moodle 5.1+ by gthomas2 · Pull Request #114 · citricity/mchef

gthomas2 · 2026-03-09T20:17:01Z

No description provided.

Copilot

Pull request overview

This PR aims to improve the experience of launching DBeaver from the database command (Issue #113) by generating a “connect” command and running GUI clients in a detached way (with extra macOS activation). It also updates PHPUnit/Behat init commands to optionally account for Moodle’s public/ folder layout.

Changes:

Refactors DBeaver command generation to dbeaverConnectCommand() and updates callers/tests accordingly.
Adds execDetached() and uses it when launching GUI database clients, with an extra macOS AppleScript activation for DBeaver.
Updates Behat/PHPUnit init paths to include public/ when needed via Moodle::shouldUsePublicFolder().

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/Traits/ExecTrait.php	Adds `execDetached()` for running commands in the background.
src/Command/Database.php	Uses `dbeaverConnectCommand()` and runs GUI clients detached; adds macOS DBeaver activation.
src/Database/AbstractDatabase.php	Centralizes DBeaver connect command building across DB implementations.
src/Database/Mysql.php	Moves DBeaver connection-string building into a base method used by AbstractDatabase.
src/Database/Postgres.php	Moves DBeaver connection-string building into a base method used by AbstractDatabase.
src/Database/DatabaseInterface.php	Renames the DBeaver API method to `dbeaverConnectCommand()`.
src/Tests/DatabaseConnectionStringsTest.php	Updates tests to call the renamed DBeaver method.
src/Command/PHPUnit.php	Uses Moodle service to optionally include `public/` for phpunit init path.
src/Command/Behat.php	Uses Moodle service to optionally include `public/` for behat init path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-09T20:20:52Z

src/Traits/ExecTrait.php

+        // Execute command in background to detach from PHP process
+        // This allows GUI applications to get proper focus
+        exec($cmd . ' &');


execDetached() appends & unconditionally, which is POSIX-shell specific. This will not detach correctly on Windows (and can even change the meaning of the command because & is a command separator in cmd.exe). Consider making detaching OS-aware (e.g., start "" ... on Windows; nohup ... >/dev/null 2>&1 & on Unix) and propagate errors when the shell fails to launch the process.

Suggested change

// Execute command in background to detach from PHP process

// This allows GUI applications to get proper focus

exec($cmd . ' &');

// Execute command in background to detach from PHP process.

// This is made OS-aware to work correctly on both Unix-like systems and Windows.

if (stripos(PHP_OS_FAMILY, 'Windows') === 0) {

// On Windows, use "start" to launch a detached process.

// The empty title ("") is required to avoid treating the first argument as the window title.

$detachedCmd = 'start "" ' . $cmd;

} else {

// On Unix-like systems, use nohup and redirect all output away, then background.

$detachedCmd = 'nohup ' . $cmd . ' >/dev/null 2>&1 &';

}

$output = [];

$returnVar = 0;

exec($detachedCmd, $output, $returnVar);

if ($returnVar !== 0) {

throw new ExecFailed("Exec failed (detached): $detachedCmd (exit $returnVar)", 0, $detachedCmd);

}

Copilot · 2026-03-09T20:20:53Z

src/Command/Database.php

+                $this->execDetached($cmd);
+
+                if (OS::isMac() && $client === 'dbeaver') {
+                    $applescript = 'tell application "DBeaver" to activate';
+                    $this->execDetached('sleep 1 && osascript -e ' . escapeshellarg($applescript));


openDatabaseClient() routes GUI clients through execDetached(), but execDetached is not Windows-safe (it appends &). This means launching DBeaver/pgAdmin/Workbench from Windows is likely to fail or behave unexpectedly. Either avoid execDetached on Windows (since start already detaches) or update execDetached to handle Windows appropriately.

Suggested change

$this->execDetached($cmd);

if (OS::isMac() && $client === 'dbeaver') {

$applescript = 'tell application "DBeaver" to activate';

$this->execDetached('sleep 1 && osascript -e ' . escapeshellarg($applescript));

if (OS::isWindows()) {

// On Windows, GUI commands typically use 'start', which already detaches

$this->exec($cmd);

} else {

$this->execDetached($cmd);

if (OS::isMac() && $client === 'dbeaver') {

$applescript = 'tell application "DBeaver" to activate';

$this->execDetached('sleep 1 && osascript -e ' . escapeshellarg($applescript));

}

Copilot · 2026-03-09T20:20:53Z

src/Database/AbstractDatabase.php

+        } else if (OS::isMac()) {
+            $dbeaverBaseCmd = 'open -na "DBeaver" --args';
+        } else if (OS::isLinux()) {
+            $dbeaverBaseCmd = 'xdg-open dbeaverc';


The Linux branch uses xdg-open dbeaverc and then appends -con .... xdg-open does not forward additional CLI args to the target application, so -con will be treated as another argument to xdg-open and the connection won’t be created/opened. Use the DBeaver CLI executable directly on Linux (e.g., dbeaverc ...) or wrap the command in a shell invocation that calls the real binary with args.

Suggested change

$dbeaverBaseCmd = 'xdg-open dbeaverc';

$dbeaverBaseCmd = 'dbeaverc';

Copilot · 2026-03-09T20:20:53Z

src/Database/DatabaseInterface.php

     * Return a dbeaver connection string.
     * @return string


Docblock is now inaccurate: dbeaverConnectCommand() returns a full command to launch DBeaver (not just a connection string). Please update the description so callers understand what the method returns.

Suggested change

* Return a dbeaver connection string.

* @return string

* Return the full shell command to launch DBeaver connected to this database.

* @return string The complete DBeaver launch command.

Copilot · 2026-03-09T20:20:54Z

src/Command/PHPUnit.php

        $this->cli->notice('Initializing PHPUnit');

-        $cmd = 'docker exec -it '.$moodleContainer.' php /var/www/html/moodle/admin/tool/phpunit/cli/init.php';
+        $publicFolder = $this->moodleService->shouldUsePublicFolder($recipe) ? 'public' . DIRECTORY_SEPARATOR: '';


This builds a path inside the Linux Docker container but uses DIRECTORY_SEPARATOR. On Windows hosts this becomes \, producing an invalid in-container path like /var/www/html/moodle/public\admin/.... Use / for container paths (or use Moodle::getDockerMoodlePath() / a dedicated helper to build container paths).

Suggested change

$publicFolder = $this->moodleService->shouldUsePublicFolder($recipe) ? 'public' . DIRECTORY_SEPARATOR: '';

$publicFolder = $this->moodleService->shouldUsePublicFolder($recipe) ? 'public/' : '';

Copilot · 2026-03-09T20:20:54Z

src/Command/Behat.php

        $this->cli->notice('Initializing behat');
        $moodleContainer = $this->mainService->getDockerMoodleContainerName($instanceName);
-        $cmd = 'docker exec -it '.$moodleContainer.' php /var/www/html/moodle/admin/tool/behat/cli/init.php --axe';
+        $publicFolder = $this->moodleService->shouldUsePublicFolder($recipe) ? 'public' . DIRECTORY_SEPARATOR: '';


This builds a path inside the Linux Docker container but uses DIRECTORY_SEPARATOR. On Windows hosts this becomes \, producing an invalid in-container path like /var/www/html/moodle/public\admin/.... Use / for container paths (or use Moodle::getDockerMoodlePath() / a dedicated helper to build container paths).

Suggested change

$publicFolder = $this->moodleService->shouldUsePublicFolder($recipe) ? 'public' . DIRECTORY_SEPARATOR: '';

$publicFolder = $this->moodleService->shouldUsePublicFolder($recipe) ? 'public/' : '';

Copilot · 2026-03-09T20:20:54Z

src/Command/PHPUnit.php

 use App\Service\Main;
+use App\Service\Moodle;
 use App\Service\Plugins;
 use App\StaticVars;
 use App\Traits\ExecTrait;


PR title focuses on DBeaver focus/launching, but this change also alters PHPUnit init path resolution using Moodle::shouldUsePublicFolder(). If this is intentional, consider updating the PR title/description (or splitting into a separate PR) so reviewers can track and release these behaviors independently.

… behat for moodle 5.1+

gthomas2 requested a review from Copilot March 9, 2026 20:17

Copilot started reviewing on behalf of gthomas2 March 9, 2026 20:17 View session

Copilot AI reviewed Mar 9, 2026

View reviewed changes

gthomas2 changed the title ~~Issue #113: open dbeaver app up and focus on it~~ Issue #113: open dbeaver app up and focus on it. Also fix behat/phpunit execution with public folder for moodle 5.1+ Mar 9, 2026

Issue #113: open dbeaver app up and focus on it. Also fix phpunit and…

ca4bacc

… behat for moodle 5.1+

gthomas2 force-pushed the issue_113_open_dbclient_app branch from d1d4927 to ca4bacc Compare March 9, 2026 20:35

gthomas2 merged commit e87a9ec into main Mar 9, 2026
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #113: open dbeaver app up and focus on it. Also fix behat/phpunit execution with public folder for moodle 5.1+#114

Issue #113: open dbeaver app up and focus on it. Also fix behat/phpunit execution with public folder for moodle 5.1+#114
gthomas2 merged 1 commit intomainfrom
issue_113_open_dbclient_app

gthomas2 commented Mar 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        // Execute command in background to detach from PHP process
-        // This allows GUI applications to get proper focus
-        exec($cmd . ' &');
+        // Execute command in background to detach from PHP process.
+        // This is made OS-aware to work correctly on both Unix-like systems and Windows.
+        if (stripos(PHP_OS_FAMILY, 'Windows') === 0) {
+            // On Windows, use "start" to launch a detached process.
+            // The empty title ("") is required to avoid treating the first argument as the window title.
+            $detachedCmd = 'start "" ' . $cmd;
+        } else {
+            // On Unix-like systems, use nohup and redirect all output away, then background.
+            $detachedCmd = 'nohup ' . $cmd . ' >/dev/null 2>&1 &';
+        }
+        $output = [];
+        $returnVar = 0;
+        exec($detachedCmd, $output, $returnVar);
+        if ($returnVar !== 0) {
+            throw new ExecFailed("Exec failed (detached): $detachedCmd (exit $returnVar)", 0, $detachedCmd);
+        }

	$dbeaverBaseCmd = 'xdg-open dbeaverc';
	$dbeaverBaseCmd = 'dbeaverc';

	$publicFolder = $this->moodleService->shouldUsePublicFolder($recipe) ? 'public' . DIRECTORY_SEPARATOR: '';
	$publicFolder = $this->moodleService->shouldUsePublicFolder($recipe) ? 'public/' : '';

Conversation

gthomas2 commented Mar 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants