Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
in #2415599: After all foreach loops that reference the value (e.g. => &$value) unset the value variable afterward we initially created a few tests but they proved to be unhelpful without building a bunch more example modules. I made them a bit more generic and split them off here. If these go well I can add some more in to cover more of the API functionality.
Comment | File | Size | Author |
---|---|---|---|
#12 | interdiff-2624700-5-12.txt | 2.53 KB | liamanderson |
#12 | basic_functional_tests-2624700-12.patch | 3.95 KB | liamanderson |
Comments
Comment #2
smccabe CreditAttribution: smccabe as a volunteer and at Acro Commerce commentedComment #3
smccabe CreditAttribution: smccabe as a volunteer and at Acro Commerce commentedmissing newlines added
Comment #4
mglamanShould we do a test to ensure callbacks generated properly?
Comment #5
joelpittetDriveby review. +1 to getting these tests started. I'd say just a tiny bit more and just commit it and work from there.
Little nits: end with period, apt and space before first word.
Left over I presume?
Agree with @mglaman, we should check we are getting more than nonNull
Comment #6
liamanderson CreditAttribution: liamanderson at Acro Commerce commentedI've integrated the suggested changes in comments #4 and #5. Can someone please check that the test for the callback is good?
Comment #8
liamanderson CreditAttribution: liamanderson at Acro Commerce commentedForgot to add success message to assertTrue(s).
Comment #10
smccabe CreditAttribution: smccabe as a volunteer and at Acro Commerce commentedTest addition looks good to me, I wrote the first part so I probably can't RTBC, @joelpittet?
Comment #11
joelpittetA slightly more thorough review(yet still quick as hell):
Is all really needed or maybe we can speed this test up by slimming down to minimum until those other modules are needed?
A docblock comment above the test functions to summarize what you are testing with these tests would help.
This may be good enough for the docblock instead?
If I recall correctly, we shouldn't be translating assertions. @see the note on this page for reference: https://www.drupal.org/node/265828
nit: 80 chars wrap.
Nit: end line break before ending class curly
Comment #12
liamanderson CreditAttribution: liamanderson at Acro Commerce commentedI have updated the patch with the suggestions in comment #11 with the exception of #1. I am not sure what modules we could slim this down to.
I have also updated the patch name based on the suggestion from Dreditor.