Skip to content

[apex] ApexUnitTestClassShouldHaveAssertsRule: Support new Assert class (Apex v56.0)#4097

Merged
adangel merged 20 commits into
pmd:masterfrom
tprouvot:feature/addNewClassForAssertion
Aug 29, 2022
Merged

[apex] ApexUnitTestClassShouldHaveAssertsRule: Support new Assert class (Apex v56.0)#4097
adangel merged 20 commits into
pmd:masterfrom
tprouvot:feature/addNewClassForAssertion

Conversation

@tprouvot

@tprouvot tprouvot commented Aug 17, 2022 •
edited by adangel
Loading

Copy link
Copy Markdown
Contributor

Describe the PR

Apex v56 introduced a new Assert class: https://developer.salesforce.com/docs/atlas.en-us.240.0.apexref.meta/apexref/apex_class_System_Assert.htm#apex_class_System_Assert

Related issues

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by github actions)
  • Added (in-code) documentation (if needed)

@tprouvot tprouvot mentioned this pull request Aug 17, 2022
Closed
@SCWells72

SCWells72 commented Aug 17, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hi, @tprouvot. The product changes look good. You should also update ApexUnitTestClassShouldHaveAsserts.xml with positive test cases to verify that usages of these new assertion methods result in no reported violations.

@tprouvot

tprouvot commented Aug 18, 2022

Copy link
Copy Markdown
Contributor Author

Hi, @tprouvot. The product changes look good. You should also update ApexUnitTestClassShouldHaveAsserts.xml with positive test cases to verify that usages of these new assertion methods result in no reported violations.

Hi @SCWells72,

Thanks for the suggestion !
One of the negative test became ... positive with the new class, thats why I updated it also.

SCWells72
SCWells72 suggested changes Aug 18, 2022
View reviewed changes

@SCWells72 SCWells72 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a couple of small things. Also, I'm not sure if I'll have authority to approve this for merge, but I'll at least try to provide sufficient feedback so that someone who can has less work to do to get to that point.

Comment thread ...net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestClassShouldHaveAssertsRule.java Outdated
Comment thread .../net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestClassShouldHaveAsserts.xml Outdated
@tprouvot tprouvot force-pushed the feature/addNewClassForAssertion branch from 2a22c60 to dc03e52 Compare August 18, 2022 15:09
SCWells72
SCWells72 suggested changes Aug 18, 2022
View reviewed changes

@SCWells72 SCWells72 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think with the one requested change here it's fine IMO.

Comment thread .../net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestClassShouldHaveAsserts.xml Outdated
@adangel adangel self-requested a review August 24, 2022 10:21
@adangel adangel changed the title [apex] ApexUnitTestClassShouldHaveAssertsRule : Add new assert methods for api 56 [apex] ApexUnitTestClassShouldHaveAssertsRule: Support new Assert class (Apex v56.0) Aug 24, 2022
adangel
adangel requested changes Aug 24, 2022
View reviewed changes

@adangel adangel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR!

Please have a look at my comments.
I think, we can edit #4096 to cover both rules and improve both rules with this PR.

Comment thread ...net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestClassShouldHaveAssertsRule.java Outdated
Comment thread ...net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestClassShouldHaveAssertsRule.java Outdated
Comment thread .../net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestClassShouldHaveAsserts.xml Outdated
Comment thread .../net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestClassShouldHaveAsserts.xml Outdated
Comment thread .../net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestClassShouldHaveAsserts.xml Outdated
@adangel

adangel commented Aug 24, 2022

Copy link
Copy Markdown
Member

@SCWells72 Thanks for the initial review!

@ghost

ghost commented Aug 24, 2022

Copy link
Copy Markdown
1 Message
📖 Compared to master:
This changeset changes 0 violations,
introduces 0 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 37 errors and 6 configuration errors.
Full report

Generated by 🚫 Danger

tprouvot and others added 11 commits August 25, 2022 10:46
@tprouvot @adangel
Update pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/best…
e0b9965 
…practices/ApexUnitTestClassShouldHaveAssertsRule.java

Co-authored-by: Andreas Dangel <andreas.dangel@adangel.org>
@tprouvot @adangel
Update pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule…
d5098ca 
…/bestpractices/xml/ApexUnitTestClassShouldHaveAsserts.xml

Co-authored-by: Andreas Dangel <andreas.dangel@adangel.org>
@tprouvot
Fix typo
50641a5
@tprouvot
PR review comment
2e9f25c
@tprouvot
Add new Assert methods to ApexAssertionsShouldIncludeMessageRule
c61108c
@tprouvot
Merge branch 'pmd:master' into feature/addNewClassForAssertion
d7cf959
@tprouvot
Merge branch 'feature/addNewClassForAssertion' of https://github.com/…
46aa491 
…tprouvot/pmd into feature/addNewClassForAssertion
@tprouvot
Add tests for new methods
d6194be
@tprouvot
Update add violation parameters to macth with new tests
ac94df9
@tprouvot
Remove param message not mandatory in this test
065b4b8
@tprouvot
Ad new tests for Assert method class
d73cdcc
@tprouvot

tprouvot commented Aug 25, 2022

Copy link
Copy Markdown
Contributor Author

Thanks for the PR!

Please have a look at my comments. I think, we can edit #4096 to cover both rules and improve both rules with this PR.

Hi @adangel,
Thanks for your suggestions, the PR should contains the additional tests and corrections.

adangel
adangel approved these changes Aug 29, 2022
View reviewed changes

@adangel adangel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

Comment thread .../net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexAssertionsShouldIncludeMessage.xml
@adangel adangel added this to the 6.49.0 milestone Aug 29, 2022
adangel added a commit that referenced this pull request Aug 29, 2022
@adangel
[apex] Update tests (ApexUnitTestClassShouldHaveAsserts, ApexAssertio…
7c6277c 
…nsShouldIncludeMessage) #4097
adangel added a commit that referenced this pull request Aug 29, 2022
@adangel
[doc] Update release notes (#4097, #4096)
38e7e69
adangel added a commit that referenced this pull request Aug 29, 2022
@adangel
Merge pull request #4097 from tprouvot:feature/addNewClassForAssertion
54a2614 
[apex] ApexUnitTestClassShouldHaveAssertsRule: Support new Assert class (Apex v56.0) #4097
@adangel adangel merged commit 29c4e99 into pmd:master Aug 29, 2022
@tprouvot tprouvot deleted the feature/addNewClassForAssertion branch August 30, 2022 06:45
@adangel adangel mentioned this pull request Sep 1, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adangel adangel adangel approved these changes

+1 more reviewer

@SCWells72 SCWells72 SCWells72 requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

6.49.0

Development

Successfully merging this pull request may close these issues.

[apex] ApexAssertionsShouldIncludeMessage and ApexUnitTestClassShouldHaveAsserts: support new Assert class (introduced with Apex v56.0)

3 participants

@tprouvot @SCWells72 @adangel