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.

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

allow-underscore-test for CamelCaseMethod gives unexpected results #851

Closed
SanderSander opened this issue Dec 3, 2020 · 3 comments · Fixed by #852
Closed

allow-underscore-test for CamelCaseMethod gives unexpected results #851

SanderSander opened this issue Dec 3, 2020 · 3 comments · Fixed by #852

Comments

@SanderSander
Copy link
Contributor

SanderSander commented Dec 3, 2020

  • PHPMD version: 2.9.1
  • PHP Version: 7.4
  • Installation type: composer

Current Behavior

Currently when the property allow-underscore-test is set to true for the CamelCaseMethod rule, method names like test_that_something_is_ok are not accepted while documentation says "Is it allowed to have underscores in test method names."

Changing the regex in the rule to /^test[a-zA-Z0-9]*([_][a-z][a-zA-Z0-9]*)*?$/ should fix the problem.

Expected behavior

Method like test_that_something_is_ok should be allowed as described in documentation

Additional

I could make a PR myself, but in the Unit tests the following is documented:

    /**
     * Tests that the rule does apply for a test method name
     * with multiple underscores even when one is allowed.
     *
     * @return void
     */

So I'm not sure what the expected behavior would be?

@kylekatarnls
Copy link
Member

Hello,

test_that_something_is_ok matches the RegExp and does not produce violation when allow-underscore-test is true.

Please provide a minimal way to reproduce your error.

@kylekatarnls
Copy link
Member

The PHPDoc code you mention is about forbidding multiple underscores, but test_that_something_is_ok does not contain multiple underscores, so it falls to testRuleDoesNotApplyForTestMethodWithUnderscoreWhenAllowed which is a o-violation test.

@SanderSander
Copy link
Contributor Author

SanderSander commented Dec 3, 2020

The PHPDoc code you mention is about forbidding multiple underscores, but test_that_something_is_ok does not contain multiple underscores, so it falls to testRuleDoesNotApplyForTestMethodWithUnderscoreWhenAllowed which is a o-violation test.

I misinterpreted the documentation in the unit test I think.... Do you mean a method name like test_hello__oops

I did some more research but it really isn't working ;) You can test this yourself by changing the file src/test/resources/files/Rule/Controversial/CamelCaseMethodName/testRuleDoesNotApplyForTestMethodWithUnderscoreWhenAllowed.php and change the function name there.

The test will fail if you use the method name test_that_something_is_ok.

My ruleset.xml

<?xml version="1.0"?>
<ruleset name="PHPMD rule set"
         xmlns="http://pmd.sf.net/ruleset/1.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://pmd.sf.net/ruleset/1.0.0 http://pmd.sf.net/ruleset_xml_schema.xsd"
         xsi:noNamespaceSchemaLocation="http://pmd.sf.net/ruleset_xml_schema.xsd">

    <rule ref="rulesets/controversial.xml">
        <exclude name="CamelCaseMethodName" />
    </rule>
    <rule ref="rulesets/controversial.xml/CamelCaseMethodName">
        <properties>
            <property name="allow-underscore-test" value="true" />
        </properties>
    </rule>
</ruleset>

Furthermore in the test testRuleAppliesToTestMethodWithTwoUnderscoresEvenWhenOneIsAllowed the method with name testGivenSomeValue_expectSome_niceResult is tested which I expect to be a valid method name and not an invalid method name because like you said it doesn't contain multiple underscores.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants