Skip to content

Simplify test suite configuration #4067

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

Merged
merged 1 commit into from
Jun 12, 2020

Conversation

morozov
Copy link
Member

@morozov morozov commented Jun 12, 2020

Q A
Type improvement
BC Break no
  1. None of the supported drivers require all the connection parameters required by the test suite configuration. Most of them have default values for most of the parameters (e.g. mysqli can connect even w/o any parameters). Having to specify them all in a local development environment is unnecessarily tedious.
  2. All so-called temporary connections look identical to the primary ones with the exception that the temporary one doesn't usually have a database name specified (creating a database is the primary purpose of the temporary connection). Instead of requiring the parameters to be specified explicitly, in the absence of the parameters, the test suite can fall back to the primary connection parameters and ignore the database.
  3. There's no exact match between the test suite configuration parameter names and the connection parameters. E.g. the the driver is configured as db_type, dbname is configured as db_name. It would be nice to have a direct correspondence.

BTW, tmp_host, tmp_port, etc. also look redundant.

Change summary

  1. (tmp)db_type(tmp)db_driver
  2. (tmp)db_username(tmp)db_user. This isn't nice but it corresponds to the connection parameter name.
  3. (tmp)db_name(tmp)db_dbname. This isn't nice either, but this is because we're not configuring a database (db_), we're configuring a test database connection. We may get rid of the db_ prefix in parameter names or replace it with something else (e.g. test_).
  4. Connection parameters are reordered logically: driver, host, port, user, password.

@morozov morozov force-pushed the simplify-test-configuration branch 5 times, most recently from ebfe221 to 2bf8fff Compare June 12, 2020 02:57
@codecov-commenter
Copy link

codecov-commenter commented Jun 12, 2020

Codecov Report

Merging #4067 into 2.10.x will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             2.10.x    #4067      +/-   ##
============================================
- Coverage     73.32%   73.30%   -0.03%     
  Complexity     5031     5031              
============================================
  Files           215      215              
  Lines         12825    12825              
============================================
- Hits           9404     9401       -3     
- Misses         3421     3424       +3     
Impacted Files Coverage Δ Complexity Δ
lib/Doctrine/DBAL/Driver/SQLSrv/Driver.php 84.21% <0.00%> (-5.27%) 9.00% <0.00%> (ø%)
lib/Doctrine/DBAL/Driver/PDOSqlsrv/Driver.php 92.59% <0.00%> (-3.71%) 13.00% <0.00%> (ø%)
lib/Doctrine/DBAL/Driver/PDOPgSql/Driver.php 84.61% <0.00%> (-2.57%) 20.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ad4c16...f88dc28. Read the comment docs.

@morozov morozov force-pushed the simplify-test-configuration branch 2 times, most recently from f813945 to 78a4452 Compare June 12, 2020 03:12
@morozov morozov force-pushed the simplify-test-configuration branch from 78a4452 to f88dc28 Compare June 12, 2020 03:36
@morozov
Copy link
Member Author

morozov commented Jun 12, 2020

The code coverage slightly dropped because the pdo_pgsql and *sqlsrv drivers no longer use the port to build the DSN. It's better to test building DSN later in a unit test rather than have it in the connection parameters to have unintended coverage.

@morozov morozov merged commit 3745877 into doctrine:2.10.x Jun 12, 2020
@morozov morozov deleted the simplify-test-configuration branch June 12, 2020 13:24
@morozov morozov self-assigned this Jun 12, 2020
@morozov morozov added this to the 2.10.3 milestone Jun 12, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants