Skip to content

fix: test environment setup for clean Docker installs#115

Closed
JpMaxMan wants to merge 1 commit intomainfrom
fix/test-env-seeder-setup
Closed

fix: test environment setup for clean Docker installs#115
JpMaxMan wants to merge 1 commit intomainfrom
fix/test-env-seeder-setup

Conversation

@JpMaxMan
Copy link
Contributor

What

Two fixes that make the test suite work correctly in clean Docker environments (i.e. when someone clones the repo fresh and runs the tests for the first time).

1. Fix: seed groups API and scopes in TestSeeder

OAuth2GroupApiTest requests the groups/read-all and groups/write scopes during its OAuth2 flow. These scopes are created by a data migration (Version20250731141145), but OpenStackIDBaseTestCase wipes oauth2_api_scope before every test run and re-seeds from TestSeeder — which did not include these scopes.

In a clean environment where migrations are run before the tests, the migration is already recorded as applied, so it does not re-run during the test, and the scopes are never restored. Result: 400 on the token endpoint, test fails.

Fixed by adding the groups API and its two scopes to TestSeeder directly, following the same pattern used for other APIs.

2. Docs: add .env.testing.example and testing instructions

When PHPUnit runs, it sets APP_ENV=testing (via phpunit.xml), causing Laravel to load .env.testing instead of .env. This is not documented anywhere, and the missing file produces no clear error — it just silently falls back to .env, which is missing many vars the OAuth2 server depends on.

Added:

  • .env.testing.example — pre-configured for the standard Docker Compose setup, ready to copy
  • A "Running the Test Suite" section in LOCAL_DEVELOPMENT_HOWTO.md explaining the .env.testing requirement and the stopOnFailure behavior
  • Updated .gitignore to allow .env.testing.example to be committed despite the .env.* rule

Why

Both issues caused OAuth2GroupApiTest::testGetAll to fail (returning 400 instead of 200) in any clean Docker setup. The root causes were non-obvious and would affect any new contributor setting up the project for the first time.

- Add .env.testing.example pre-configured for Docker Compose local dev
- Document .env.testing requirement and stopOnFailure behavior in LOCAL_DEVELOPMENT_HOWTO.md
- Update .gitignore to allow .env.testing.example despite .env.* rule
- Seed groups API and scopes in TestSeeder so OAuth2GroupApiTest passes in clean environments
@JpMaxMan JpMaxMan force-pushed the fix/test-env-seeder-setup branch from 4ca7db6 to 3e95207 Compare February 28, 2026 02:38
@smarcet
Copy link
Collaborator

smarcet commented Feb 28, 2026

@JpMaxMan

The "fix" is a duplicate of existing code

The PR claims that TestSeeder doesn't seed the groups API and its scopes, causing OAuth2GroupApiTest to fail. This is wrong. The seeders already handle it:

  1. ApiSeeder (called at TestSeeder.php:448 via $this->call(ApiSeeder::class) see
    $this->call(ApiSeeder::class);
    ) already creates the groups API at ApiSeeder.php:67-74 see
    $api->setName('groups');
    .
  2. ApiScopeSeeder (called at TestSeeder.php:449 see
    $this->call(ApiScopeSeeder::class);
    ) already has a seedGroupScopes() method at ApiScopeSeeder.php:153-173 see
    'short_description' => 'Allows access to Groups info.',
    that seeds both IGroupScopes::ReadAll and IGroupScopes::Write.
  3. ApiEndpointSeeder (called at TestSeeder.php:450 see
    $this->call(ApiEndpointSeeder::class);
    ) already has seedGroupEndpoints() at ApiEndpointSeeder.php:210-222.

The full TestSeeder::run() flow calls these three seeders before the test-specific seedApis()/seedApiScopes() methods. The groups data is already seeded and nothing downstream deletes it.

also

The PR adds a groups entry to TestSeeder::seedApis(), but that method creates APIs under "test resource server", while ApiSeeder creates the groups API under "OpenStackId server". This would
create a second groups API under a different resource server.

i am gonna decline this PR , if you want the .env.testing docs, that could be a separate small PR.

@smarcet smarcet closed this Feb 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants