Streamline local test environment setup #916

schlessera · 2024-12-11T11:01:41Z

Refactored the test environment setup to make it more robust and user-friendly. This consolidates multiple test commands and improves process management for local development.

What Changed

Added shell scripts to manage test environment lifecycle
Consolidated PHPUnit commands to auto-detect version and config
Improved process management with proper PID tracking
Added mitmproxy version verification (≥11.0.2 required)
Adapted CONTRIBUTING.md to explain the new setup

The main improvement is the simplified test workflow. Instead of juggling different commands for PHPUnit versions and remembering to start/stop test servers, everything is now handled through clear, single-purpose commands.

New Scripts

scripts/start-test-environment.sh - Boots test and proxy servers - should be sourced to set environment vairables
scripts/stop-test-environment.sh - Clean shutdown of all processes
scripts/run-phpunit.sh - Version-agnostic PHPUnit runner
scripts/run-tests-with-server.sh - Full test environment run

Usage

Basic testing is now simpler:

composer test # Run tests (auto-detects PHPUnit version)
composer coverage # Same, but with coverage

For development with local test servers:

# Direct usage (sets env vars in your shell)
source scripts/start-test-environment.sh
scripts/run-phpunit.sh
scripts/stop-test-environment.sh

# Or through composer (automated start/stop)
composer test:withserver

jrfnl

Looking good.

Will still need a note in CONTRIBUTING that this will not work on Windows for now.

As discussed, not merging yet until a shellcheck check has been added in CI to safeguard the scripts.

jrfnl · 2024-12-11T12:04:21Z

Future scope: add a Docker config file to get up & running with even less effort (and which would work on Windows too).

jrfnl · 2024-12-11T12:17:56Z

Just noticed, just running the script on my machine created some .test-pids/*.pid files. You may want to add those to the .gitignore file ?

schlessera · 2024-12-11T14:04:02Z

I changed the folder where PIDs are stored and added that location to .gitignore: tests/utils/pids

jrfnl · 2025-01-22T10:29:26Z

Just checking - this PR relates to #819 (and closes it ?)

jrfnl

@schlessera I've had a good look at this PR, with the caveat of limited knowledge of bash scripting...

Left a number of questions in-line.

Additionally:

The scripts re-introduce a problem we previously solved: the system default PHP version being used instead of the Composer PHP version being used. This is a regression, which I'm not happy with.
/scripts/ directory needs to be added to .gitattributes
composer.json: script descriptions should mention that this doesn't work on Windows, only on *nix
CONTRIBUTING: should mention that this doesn't work on Windows, only on *nix.
CONTRIBUTING: "The environment scripts must be sourced"... what does that mean ?
Note: the source command is not available on Windows.

Other remarks:

The minimum versions for Python and mitmproxy will now need to be maintained in quite a few places (CONTRIBUTING, quicktest.yml, test.yml, the new scripts). This feels pretty error prone...
Just as a test, I've ran the scripts on Windows:
- run-tests-with-server.sh doesn't appears to do anything (other than flash open a new shell window & close it) and exits without giving the user information about why it's not working.
- start-test-environment.sh doesn't appears to do anything (other than flash open a new shell window & close it) and exits without giving the user information about why it's not working.
- stop-test-environment.sh doesn't appears to do anything (other than flash open a new shell window & close it) and exits without giving the user information about why it's not working.
- run-phpunit.sh runs, but opens a new shell window and closes it immediately once the test run has finished, meaning you can't read the results.
  The new window is also really annoying for other reasons, like usability issues, no scrolling, window size being tiny etc

jrfnl · 2025-01-22T10:37:47Z

.github/CONTRIBUTING.md

 ```bash
-# Start the test server
+# Just the test server


Suggested change

# Just the test server

# Start only the test server

jrfnl · 2025-01-22T10:38:08Z

.github/CONTRIBUTING.md


-# Start the proxy server
+# Just the proxy servers


Suggested change

# Just the proxy servers

# Start only the proxy servers

jrfnl · 2025-01-22T10:40:18Z

.github/workflows/quicktest.yml

        id: phpunit_version
        run: echo "VERSION=$(vendor/bin/phpunit --version | grep --only-matching --max-count=1 --extended-regexp '\b[0-9]+\.[0-9]+')" >> $GITHUB_OUTPUT


Looks like this step can now be removed ?

jrfnl · 2025-01-22T10:41:14Z

.github/CONTRIBUTING.md

-# Run the tests
-composer test


Why is this being removed here ?

jrfnl · 2025-01-22T10:42:13Z

.github/workflows/test.yml

        id: phpunit_version
        run: echo "VERSION=$(vendor/bin/phpunit --version | grep --only-matching --max-count=1 --extended-regexp '\b[0-9]+\.[0-9]+')" >> $GITHUB_OUTPUT


Dito. This step can probably be removed now.

jrfnl · 2025-01-22T10:56:34Z

scripts/run-phpunit.sh

+# Try to determine script location
+if [ -n "${BASH_SOURCE:-}" ]; then
+    # Bash-specific path resolution
+    SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+    PROJECT_ROOT="$(dirname "$SCRIPT_DIR")"
+else
+    # POSIX fallback - assume we're in the repository root
+    PROJECT_ROOT="$(pwd)"
+    SCRIPT_DIR="$PROJECT_ROOT/scripts"
+fi


Out of curiousity: why is this needed ? AFAIK Composer scripts only work from the directory containing the composer.json file, so in that case, we'd always be in the project root.

I imagine it's because people can call these scripts directly ?

jrfnl · 2025-01-22T11:00:35Z

scripts/run-tests-with-server.sh

+TEST_EXIT_CODE=$?
+
+# Stop the test environment
+. "${SCRIPT_DIR}/stop-test-environment.sh"


Shouldn't there be an error as well if the test environment couldn't be stopped cleanly ?

jrfnl · 2025-01-22T11:02:28Z

scripts/start-test-environment.sh

+
+# Check if mitmproxy is installed
+if ! command -v mitmdump >/dev/null 2>&1; then
+    echo "Error: mitmproxy is not installed. Please install it with: pip3 install mitmproxy"


Should there be a check first to see if Python/Pip3 is installed ?

jrfnl · 2025-01-22T11:04:05Z

scripts/start-test-environment.sh

Just wondering: if a user already has a server running which serves localhost, will this script cause problems with that?

jrfnl · 2025-01-22T11:06:48Z

.github/CONTRIBUTING.md

@@ -109,28 +109,65 @@ Code coverage is monitored for every PR and for the code base as a whole using [

 ### Running the Tests



Might be good to update the "Prerequisites for running the tests" section to mention the minimum versions, i.e. Python 3.12 and mitmproxy 11.0.2.

jrfnl reviewed Dec 11, 2024

View reviewed changes

schlessera added 5 commits January 21, 2025 11:55

Add scripts to simplify local testing

bbd0698

Adapt the CONTRIBUTING.md file with the testing flow

7061547

Improve POSIX compatibility

0e502fc

Use different folder for storing pids and gitignore as needed

b380380

Remove conditional PHPUnit paths from GHA workflows

f09ca3e

schlessera force-pushed the add/simplified-local-testserver branch from 60f07ce to f09ca3e Compare January 21, 2025 11:01

schlessera added 2 commits January 21, 2025 12:18

Adapt argument support in PHPUnit script

413980c

Support multiple arguments in PHPUnit helper script

bc32ca1

jrfnl mentioned this pull request Jan 22, 2025

Add a Docker config #917

Open

jrfnl requested changes Jan 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Streamline local test environment setup #916

Streamline local test environment setup #916

schlessera commented Dec 11, 2024 •

edited

Loading

jrfnl left a comment

jrfnl commented Dec 11, 2024

jrfnl commented Dec 11, 2024

schlessera commented Dec 11, 2024

jrfnl commented Jan 22, 2025 •

edited

Loading

jrfnl left a comment

jrfnl Jan 22, 2025

jrfnl Jan 22, 2025

jrfnl Jan 22, 2025

jrfnl Jan 22, 2025

jrfnl Jan 22, 2025

jrfnl Jan 22, 2025

jrfnl Jan 22, 2025

jrfnl Jan 22, 2025

jrfnl Jan 22, 2025

jrfnl Jan 22, 2025

		id: phpunit_version
		run: echo "VERSION=$(vendor/bin/phpunit --version \| grep --only-matching --max-count=1 --extended-regexp '\b[0-9]+\.[0-9]+')" >> $GITHUB_OUTPUT

		@@ -109,28 +109,65 @@ Code coverage is monitored for every PR and for the code base as a whole using [

		### Running the Tests

Streamline local test environment setup #916

Are you sure you want to change the base?

Streamline local test environment setup #916

Conversation

schlessera commented Dec 11, 2024 • edited Loading

What Changed

New Scripts

Usage

jrfnl left a comment

Choose a reason for hiding this comment

jrfnl commented Dec 11, 2024

jrfnl commented Dec 11, 2024

schlessera commented Dec 11, 2024

jrfnl commented Jan 22, 2025 • edited Loading

jrfnl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schlessera commented Dec 11, 2024 •

edited

Loading

jrfnl commented Jan 22, 2025 •

edited

Loading