Skip to content

Add required key attribute #29174

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
Nov 11, 2018

Conversation

greg0ire
Copy link
Contributor

@greg0ire greg0ire commented Nov 11, 2018

Q A
Branch? 2.8
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

I am getting warnings when running tests with recent phpunit versions (since 7.2.0) :

- Element 'element': The attribute 'key' is required but missing.

This requirement is far from being new, what is recent is phpunit
validating its configuration file against the XSD schema.

See sebastianbergmann/phpunit@d4484be

This is pedantic (not a bugfix), and might be a bit painful to merge upstream, so if you feel like I should target a more recent instead and let the old branches go on with their lives, please tell me.

I am getting warnings when running tests with recent phpunit versions:

> - Element 'element': The attribute 'key' is required but missing.

This requirement is far from being new, what is recent is phpunit
validating its configuration file against the XSD schema.

See sebastianbergmann/phpunit@d4484be
@greg0ire greg0ire force-pushed the add_require_key_attribute branch from 9dd1cb5 to c0733c2 Compare November 11, 2018 10:18
@greg0ire
Copy link
Contributor Author

greg0ire commented Nov 11, 2018

🤔 looks like there are more issues to fix:

phpunit.xml.dist:10: element phpunit: Schemas validity error : Element 'phpunit', attribute 'failOnRisky': The attribute 'failOnRisky' is not allowed.
phpunit.xml.dist:10: element phpunit: Schemas validity error : Element 'phpunit', attribute 'failOnWarning': The attribute 'failOnWarning' is not allowed.
phpunit.xml.dist fails to validate

That one is probably an issue with the XSD url being outdate, because I can see these attributes here: https://github.com/sebastianbergmann/phpunit/blob/master/phpunit.xsd

I think 5.2 should probably be used here: sebastianbergmann/phpunit@7e06a82

It validates if I use http://schema.phpunit.de/5.2/phpunit.xsd . Composer seems to say lower versions should be allowed though:

"phpunit/phpunit": "<4.8.35|<5.4.3,>=5.0"

What is the right call here? I think I should change the url.

I also checked the other phpunit config files in the project, they all validate against that version of the XSD:

wget http://schema.phpunit.de/5.2/phpunit.xsd && find src -name phpunit.xml.dist| xargs xmllint --schema phpunit.xsd  1>/dev/null 
src/Symfony/Component/Routing/phpunit.xml.dist validates
src/Symfony/Component/OptionsResolver/phpunit.xml.dist validates
src/Symfony/Component/ClassLoader/phpunit.xml.dist validates
src/Symfony/Component/Config/phpunit.xml.dist validates
src/Symfony/Component/PropertyAccess/phpunit.xml.dist validates
src/Symfony/Component/Filesystem/phpunit.xml.dist validates
src/Symfony/Component/Stopwatch/phpunit.xml.dist validates
src/Symfony/Component/DependencyInjection/phpunit.xml.dist validates
src/Symfony/Component/Intl/phpunit.xml.dist validates
src/Symfony/Component/PropertyInfo/phpunit.xml.dist validates
src/Symfony/Component/HttpKernel/phpunit.xml.dist validates
src/Symfony/Component/Form/phpunit.xml.dist validates
src/Symfony/Component/ExpressionLanguage/phpunit.xml.dist validates
src/Symfony/Component/VarDumper/phpunit.xml.dist validates
src/Symfony/Component/Console/phpunit.xml.dist validates
src/Symfony/Component/Asset/phpunit.xml.dist validates
src/Symfony/Component/Finder/phpunit.xml.dist validates
src/Symfony/Component/Locale/phpunit.xml.dist validates
src/Symfony/Component/Process/phpunit.xml.dist validates
src/Symfony/Component/CssSelector/phpunit.xml.dist validates
src/Symfony/Component/Ldap/phpunit.xml.dist validates
src/Symfony/Component/Validator/phpunit.xml.dist validates
src/Symfony/Component/DomCrawler/phpunit.xml.dist validates
src/Symfony/Component/Translation/phpunit.xml.dist validates
src/Symfony/Component/Security/Guard/phpunit.xml.dist validates
src/Symfony/Component/Security/phpunit.xml.dist validates
src/Symfony/Component/Security/Csrf/phpunit.xml.dist validates
src/Symfony/Component/Security/Http/phpunit.xml.dist validates
src/Symfony/Component/Security/Core/phpunit.xml.dist validates
src/Symfony/Component/Debug/phpunit.xml.dist validates
src/Symfony/Component/Debug/vendor/symfony/http-foundation/phpunit.xml.dist validates
src/Symfony/Component/Debug/vendor/symfony/event-dispatcher/phpunit.xml.dist validates
src/Symfony/Component/Debug/vendor/symfony/http-kernel/phpunit.xml.dist validates
src/Symfony/Component/Serializer/phpunit.xml.dist validates
src/Symfony/Component/Yaml/phpunit.xml.dist validates
src/Symfony/Component/Templating/phpunit.xml.dist validates
src/Symfony/Component/HttpFoundation/phpunit.xml.dist validates
src/Symfony/Component/EventDispatcher/phpunit.xml.dist validates
src/Symfony/Component/BrowserKit/phpunit.xml.dist validates
src/Symfony/Bridge/PhpUnit/phpunit.xml.dist validates
src/Symfony/Bridge/Doctrine/phpunit.xml.dist validates
src/Symfony/Bridge/Twig/phpunit.xml.dist validates
src/Symfony/Bridge/Monolog/phpunit.xml.dist validates
src/Symfony/Bridge/ProxyManager/phpunit.xml.dist validates
src/Symfony/Bundle/FrameworkBundle/phpunit.xml.dist validates
src/Symfony/Bundle/WebProfilerBundle/phpunit.xml.dist validates
src/Symfony/Bundle/DebugBundle/phpunit.xml.dist validates
src/Symfony/Bundle/TwigBundle/phpunit.xml.dist validates
src/Symfony/Bundle/SecurityBundle/phpunit.xml.dist validates

@nicolas-grekas nicolas-grekas added this to the 2.8 milestone Nov 11, 2018
@nicolas-grekas
Copy link
Member

Let's change the URL yes. Since lowest versions of phpunit don't care about it, there is no issue doing so.

@greg0ire
Copy link
Contributor Author

I will make a separate PR for this, since there are a lot more files to change.

@greg0ire
Copy link
Contributor Author

See #29175

@nicolas-grekas
Copy link
Member

Thank you @greg0ire.

@nicolas-grekas nicolas-grekas merged commit c0733c2 into symfony:2.8 Nov 11, 2018
nicolas-grekas added a commit that referenced this pull request Nov 11, 2018
This PR was merged into the 2.8 branch.

Discussion
----------

Add required key attribute

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

I am getting warnings when running tests with recent phpunit versions (since 7.2.0) :

> `- Element 'element': The attribute 'key' is required but missing.`

This requirement is far from being new, what is recent is phpunit
validating its configuration file against the XSD schema.

See sebastianbergmann/phpunit@d4484be

This is pedantic (not a bugfix), and might be a bit painful to merge upstream, so if you feel like I should target a more recent instead and let the old branches go on with their lives, please tell me.

Commits
-------

c0733c2 Add required key attribute
@greg0ire greg0ire deleted the add_require_key_attribute branch November 11, 2018 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants