Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Testing Guidelines #4356

Merged
merged 1 commit into from
Nov 8, 2020
Merged

Testing Guidelines #4356

merged 1 commit into from
Nov 8, 2020

Conversation

morozov
Copy link
Member

@morozov morozov commented Oct 18, 2020 •
edited
Loading

Q A
Type improvement
BC Break no

This is a placeholder and a temporary reference for the testing requirements and guidelines.

TODO:

  • Add more details and examples to facilitate decision making.
  • Highlight the importance of testing on all platforms rather than on a specific one where the issue being fixed was observed. In addition to making sure that all platforms implement the same logic, they can be used as a reference for using the DBAL API.
  • Document the cases that may be exempt from automated integration testing (e.g. transactions, locks, race conditions, etc).
  • If a test covers a fix, it should fail without the fix.
  • Add a link to the sidebar.

@SenseException
Copy link
Member

SenseException commented Oct 19, 2020 •
edited
Loading

In its current state it would also fit well to other projects than dbal. Could this be used as a generic guideline on the website or will this diverge more from the other projects (except platforms)?

@morozov
Copy link
Member Author

morozov commented Oct 19, 2020

My goal is to have the DBAL documentation first. Once it's done content-wise, we can see how it could be restructured into the generic and DBAL-specific sections. I'm open to suggestions and contributions.

@morozov morozov force-pushed the testing-guidelines branch 3 times, most recently from 0e93fe3 to 03d29b6 Compare October 21, 2020 02:00
@morozov
Copy link
Member Author

morozov commented Oct 21, 2020

I could use some help with the examples (if needed) and feedback on whether some ideas need to be clarified.

greg0ire
greg0ire reviewed Oct 22, 2020
View reviewed changes
docs/en/reference/testing.rst Outdated Show resolved Hide resolved
docs/en/reference/testing.rst Show resolved Hide resolved
docs/en/reference/testing.rst Outdated Show resolved Hide resolved
docs/en/reference/testing.rst Outdated Show resolved Hide resolved
docs/en/reference/testing.rst Outdated Show resolved Hide resolved
docs/en/reference/testing.rst Outdated
with other units. In this case, the other units could be mocked.

Unit tests are mostly welcomed for testing the logic that the DBAL itself defines (e.g. logging). They also could be
used as a secondary tier of testing for the logic that requires integration testing.
Copy link
Member

Choose a reason for hiding this comment

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

That last sentence could be clarified with an example.

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved this under the "Using Unit and Integration Tests Together" section. The wording is to be revisited.

docs/en/reference/testing.rst Outdated Show resolved Hide resolved
docs/en/reference/testing.rst Outdated Show resolved Hide resolved
@morozov morozov mentioned this pull request Nov 7, 2020
Closed
@morozov morozov changed the title Testing guidelines Testing Guidelines Nov 8, 2020
@morozov morozov changed the base branch from 3.0.x to 2.12.x November 8, 2020 15:07
@morozov
Copy link
Member Author

morozov commented Nov 8, 2020 •
edited
Loading

I haven't been finding much passion lately to finish this document, so in order to not let it rot, let's give it another round of proofreading and try to not increase the scope.

The primary idea I want to convey is the need to write integration tests across all supported platforms as I have been asking contributors dozens and dozens of times. Everybody can contribute to this guide afterward and add the other important bits.

@morozov morozov marked this pull request as ready for review November 8, 2020 15:27
morozov
morozov commented Nov 8, 2020
View reviewed changes
docs/en/reference/testing.rst Outdated Show resolved Hide resolved
morozov
morozov commented Nov 8, 2020
View reviewed changes
docs/en/reference/testing.rst Outdated Show resolved Hide resolved
@morozov morozov added this to the 2.12.1 milestone Nov 8, 2020
greg0ire
greg0ire previously approved these changes Nov 8, 2020
View reviewed changes
@morozov morozov mentioned this pull request Nov 8, 2020
Merged
SenseException
SenseException requested changes Nov 8, 2020
View reviewed changes
Copy link
Member

@SenseException SenseException left a comment

Choose a reason for hiding this comment

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

I haven't been finding much passion lately to finish this document, so in order to not let it rot, let's give it another round of proofreading and try to not increase the scope.

Okay 👍 . Just have a small change, otherwise this is ready for a merge IMO.

docs/en/reference/testing.rst Outdated Show resolved Hide resolved
@SenseException SenseException merged commit d31c697 into doctrine:2.12.x Nov 8, 2020
@morozov morozov deleted the testing-guidelines branch November 10, 2020 23:20
@morozov
Copy link
Member Author

morozov commented Nov 10, 2020

@SenseException how do we get this deployed to the website?

rgrellmann added a commit to Rossmann-IT/dbal that referenced this pull request Mar 7, 2021
@rgrellmann
Merge tag '2.12.1' into oracle122
42fbc70 
Release [2.12.1](https://github.com/doctrine/dbal/milestone/84)

2.12.1
======

- Total issues resolved: **2**
- Total pull requests resolved: **11**
- Total contributors: **7**

Documentation,Prepared Statements
---------------------------------

 - [4424: Mark SQLParserUtils internal](doctrine#4424) thanks to @morozov

Packaging
---------

 - [4416: Update .gitattributes](doctrine#4416) thanks to @bytestream

Bug,Cache
---------

 - [4414: ResultCacheStatement::fetchAllAssociative does not store results in cache](doctrine#4414) thanks to @morozov and @dFayet

Deprecation,Prepared Statements
-------------------------------

 - [4411: Deprecate inappropriate usage of prepared statement parameters](doctrine#4411) thanks to @morozov
 - [4407: Deprecate colon prefix for prepared statement parameters](doctrine#4407) thanks to @morozov

Static Analysis
---------------

 - [4403: Remove redundant phpstan param from DriverManager::getConnection()](doctrine#4403) thanks to @simPod

Bug,Locking,Transactions
------------------------

 - [4400: LockMode::NONE should not set WITH (NOLOCK)](doctrine#4400) thanks to @BenMorel

Code Style,PHP
--------------

 - [4398: Update PHP&doctrine#95;CodeSniffer to 3.5.8](doctrine#4398) thanks to @morozov

PDO,PHP,Test Suite
------------------

 - [4396: Fix php8 mysql mariadb](doctrine#4396) thanks to @greg0ire

Documentation
-------------

 - [4390: Fix headline in the upgrade docs](doctrine#4390) thanks to @jdreesen

Documentation,Testing
---------------------

 - [4356: Testing Guidelines](doctrine#4356) thanks to @morozov

# gpg: Signature made Sat Nov 14 21:50:01 2020
# gpg:                using DSA key 1BEDEE0A820BC30D858F9F0C2C3A645671828132
# gpg: Can't check signature: No public key

# Conflicts:
#	README.md
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@SenseException SenseException SenseException approved these changes

@greg0ire greg0ire greg0ire approved these changes

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
2.12.1
Development

Successfully merging this pull request may close these issues.
3 participants
@morozov @SenseException @greg0ire