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

Update api version to 56.0 and test classes to use new Assert class #436

Conversation

chazwatkins
Copy link

@chazwatkins chazwatkins commented Dec 2, 2022

This change is Reviewable

@ImJohnMDaniel
Copy link
Contributor

G'day @chazwatkins -- Just wanted to say again thank you very much for this PR. There is a lot that you updated and it is very much appreciated. Cheers!

Copy link
Contributor

@ImJohnMDaniel ImJohnMDaniel left a comment

Choose a reason for hiding this comment

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

G'day @chazwatkins. I noticed that fflib_SObjectDomain.TestSObjectStatefulDomain and fflib_SObjectDomain.TestSObjectDomain each have an assertion in the old style. Was this deliberate?

@ImJohnMDaniel
Copy link
Contributor

Also, @chazwatkins, will you be updating the class xml files to v56.0? If not, no worries. I can definitely do it. I just didn't want to double up on the work.

Thanks again!!

@chazwatkins
Copy link
Author

Since I was updating all the repos, I managed to forget to do the xml files on this one. I'll do this now.

@chazwatkins
Copy link
Author

The xml files are now all 56.0

@chazwatkins
Copy link
Author

G'day @chazwatkins. I noticed that fflib_SObjectDomain.TestSObjectStatefulDomain and fflib_SObjectDomain.TestSObjectDomain each have an assertion in the old style. Was this deliberate?

@ImJohnMDaniel It wasn't. I'll check on them and submit the update

@chazwatkins
Copy link
Author

G'day @chazwatkins. I noticed that fflib_SObjectDomain.TestSObjectStatefulDomain and fflib_SObjectDomain.TestSObjectDomain each have an assertion in the old style. Was this deliberate?

@ImJohnMDaniel It wasn't. I'll check on them and submit the update

fflib_SObjectDomain.TestSObjectStatefulDomain and fflib_SObjectDomain.TestSObjectDomain are updated to use System.Assert.areEqual now

ImJohnMDaniel
ImJohnMDaniel previously approved these changes Dec 4, 2022
Copy link
Contributor

@stohn777 stohn777 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 40 files reviewed, 1 unresolved discussion (waiting on @chazwatkins and @daveespo)


sfdx-source/apex-common/test/classes/fflib_SObjectDescribeTest.cls line 49 at r3 (raw file):

		System.Assert.isNull(fflib_SObjectDescribe.getDescribe(nullType));
		System.Assert.isNull(fflib_SObjectDescribe.getDescribe(nullDescribe));
		System.Assert.isNull(fflib_SObjectDescribe.getDescribe(nullSObject));

Should all of these use the Assert.areEqual(null, ...) pattern too? I cannot see how they would ever be insufficient, BUT ...

@chazwatkins
Copy link
Author

Reviewable status: 0 of 40 files reviewed, 1 unresolved discussion (waiting on @chazwatkins and @daveespo)

sfdx-source/apex-common/test/classes/fflib_SObjectDescribeTest.cls line 49 at r3 (raw file):

		System.Assert.isNull(fflib_SObjectDescribe.getDescribe(nullType));
		System.Assert.isNull(fflib_SObjectDescribe.getDescribe(nullDescribe));
		System.Assert.isNull(fflib_SObjectDescribe.getDescribe(nullSObject));

Should all of these use the Assert.areEqual(null, ...) pattern too? I cannot see how they would ever be insufficient, BUT ...

Yes, they should be. Thanks for catching that. I double-checked for other isNull and isNotNull occurrences and updated them to areEqual(null, ...) and areNotEqual(null, ...)

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

Successfully merging this pull request may close these issues.

3 participants