Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Comment | File | Size | Author |
---|---|---|---|
#20 | 3061563-20.patch | 4.28 KB | shubham.prakash |
| |||
#17 | 3061563-17.patch | 728 bytes | shubham.prakash |
| |||
#16 | convert_to_phpunit-3061563-16.patch | 728 bytes | MahtabAlam |
| |||
#13 | interdiff.txt | 4.06 KB | zeuty |
#13 | convert_to_phpunit-3061563-13.patch | 25.75 KB | zeuty |
|
Comments
Comment #2
JeroenTStarted working on converting the simpletests to PHPUnit tests. This still needs a lot of work.
Comment #4
BerdirThis overlaps with https://www.drupal.org/project/pathauto/issues/3042582
Comment #5
zeutyComment #6
zeutyConverted the tests.
Please review.
Comment #7
BerdirGreat work on this.
I think this implicitly relies on it being too slow to do the machine-name conversion automatically and then fills out the now-visible field. That seems tricky and could possible result in random fails.
We had the same in paragraphs, there we did it like this:
We should be able to do something similar here too?
wondering why the old assertions were commented out?
Is it because we rely on a number field that ensures that with client-side validation?
not sure we need to keep that test at all then, this just doesn't seem very useful.
If anything, we could maybe assert that the specific field we're look for exists and has a min="1" attribute?
Comment #8
zeuty@Berdir, thanks for review, I'll continue working on this.
Regarding the commented out strings - they were commented by you in 2014 :)
I just removed the commented our strings from the code.
Will update the patch soon.
Comment #9
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedNote that #2756703: URL Alias not saving in some cases partial converts the test
PathautoNodeWebTest::testCustomAliasWithoutPattern
to phpunit.Comment #10
zeutyAdjusted according to remarks from @berdir in #7
Comment #11
zeutyComment #12
zeutySorry, wrong patchfile
Comment #13
zeutyComment #15
BerdirThanks!
Comment #16
MahtabAlamCall to deprecated method strtolower() will be also removed before Drupal 9.0.0. Use mb_strtolower() instead.
Comment #17
shubham.prakash CreditAttribution: shubham.prakash at OpenSense Labs commentedRE: Call to deprecated method strtolower() will be also removed before Drupal 9.0.0. Use mb_strtolower() instead.
Hope this patch fixes the issue.
Comment #18
MahtabAlam@shubham what is the difference between the patch which I provided and yours?
Comment #19
shubham.prakash CreditAttribution: shubham.prakash at OpenSense Labs commented@MahtabAlam the patch in #16 given by you was displaying "Checkout Error" earlier, so i put my patch for the same issue. Now that, it is showing "46 pass", there is no need for patch in #17.
Comment #20
shubham.prakash CreditAttribution: shubham.prakash at OpenSense Labs commentedAlso, this patch can be applied which fixes all these deprecated methods - strlen(), strtolower(), substr(), strpos().
Comment #21
BerdirThis issue was about tests and has been committed, don't reopen it for other changes.
AFAIK there are existing issues for the mbstring functions.
Comment #23
penyaskitoNot sure if this was on purpose, but maybe the Trait should have been deprecated AND moved instead of moved.
#3087523: PathautoTestHelperTrait namespace has changed
Sadly my dependence was on * instead of -dev and I didn't see this before the release :-(
Comment #24
BerdirSorry about that. Unlike core, I don't really consider our test classes and traits to be an API. Note that depending on a trait or base class for a dev-dependency only is actually pretty problematic as it can AFAIK still kill test disccovery when you try to run tests and that dependency isn't around.