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.
Problem/Motivation
#2004690: taxonomy page broken inspired us to write tests for all the forms where we add a translate link into the operations.
#1971384: [META] Convert page callbacks to controllers Drupal Code Sprint Kyiv 2013
Proposed resolution
Add tests.
Remaining tasks
- Do one test first.
- Get a review.
- Do the rest.
User interface changes
No.
API changes
No.
Related
- noticed this while working here #2004780: Unify menu item access checking with mock menu items
- noticed this too #2004690: taxonomy page broken
Comment | File | Size | Author |
---|---|---|---|
#36 | test_all_lists-2004710-36.patch | 6.56 KB | YesCT |
#36 | interdiff-34-36.txt | 4 KB | YesCT |
#34 | test_all_lists-2004710-35.patch | 6.66 KB | kfritsche |
#34 | interdiff-31-34.txt | 8.4 KB | kfritsche |
#31 | test_all_lists-2004710-31.patch | 6.26 KB | kfritsche |
Comments
Comment #1
Gábor Hojtsy#1998600: Block config translation fails also needs test coverage yeah. We can write those tests here at once I think. I believe we only test views translatability and not all of the UI steps there either in the navigation chain.
Comment #2
YesCT CreditAttribution: YesCT commentedcurrent idea:
look for:
Comment #3
YesCT CreditAttribution: YesCT commentedtests were taking a long time to run, so I just took my one little one out and put it in a different class.
it runs much faster. can be moved back in later after it's nicer.
fails locally on taxonomy.
next: try it on a listing that works like menu and make sure it passes.
because it is a copy, it has extra permissions and such... for now.
Comment #5
YesCT CreditAttribution: YesCT commentedI'm having trouble getting a test that would pass for menus. Here is what I have so far.
The verbose from running test locally, shows *no* translate operation in the drop-buttons... uh, and I don't know why it's different in the test than locally on a site install. Permission for config_translation looks to be being added ok.
Comment #7
YesCT CreditAttribution: YesCT commentedcode improvements and trying the menu translate link directly.
Comment #9
YesCT CreditAttribution: YesCT commentedTried looking at views... still no translate link in the drop button. So @xjm recommended seeing if profile made a difference and then trying as user 1. So about to do that now.
Here is where I am for now. ... Organized each of the tests that we will need to have, into sections... alphabetically.
Comment #10
YesCT CreditAttribution: YesCT commentedexample from moduleapitest
on how to force using standard profile.
Comment #11
YesCT CreditAttribution: YesCT commentedNote to self... turning of xdebug makes things run much faster.
turning on standard profile *does* make the translate operation show.
Next, identify what parts of standard profile are needed, add them to setup and then take the standard profile back off.
Comment #13
YesCT CreditAttribution: YesCT commentedI found I could install locally with minimal and play around with what is the difference. I found that blocks seems to have translate link in operations with minimal profile. so trying that in the tests.
Comment #15
YesCT CreditAttribution: YesCT commentedin the tests, the blocks list is empty.
here block module and permission are added.
attempted to place an existing block, just guessing/grepping for id and machine name.
strange errors in the test results, but I'm taking a break from this.
Comment #16
YesCT CreditAttribution: YesCT commentedI forgot to take out the word "for". "Tests" should be a verb.
....
needs work ... for other more important things.
Comment #17
victor-shelepen CreditAttribution: victor-shelepen commentedComment #18
YesCT CreditAttribution: YesCT commentedlikin, I'm going to try something @larowlan showed me from #1869476: Convert global menus (primary links, secondary links) into blocks
$this->drupalPlaceBlock('menu_navigation', array('region' => 'secondary_menu'), array('menu' => 'account'));
ping me in irc if you have some work to post before I give it a try.
Comment #19
victor-shelepen CreditAttribution: victor-shelepen commentedThis patches confuse me. Because they are not applicable. Could anybody filter the content of the issue and clarify the task?
Comment #20
YesCT CreditAttribution: YesCT commentedIt was a while since I worked on this too. :) After a few minutes I remembered to apply it to the git clone of the config_translation project which is in modules directory. :) Then it applies.
@tim.plunkett picked out an easier block to add.
here it is.
I ran the test locally and I can see the block added to the block list *and* Translate in the dropbutton. So we can then test that Translate link is there next.
Comment #21
victor-shelepen CreditAttribution: victor-shelepen commentedTest did not pass.
http://privatepaste.com/1d970ed9aa
Comment #22
YesCT CreditAttribution: YesCT commentedyes, I'm not sure why Translate operation link does not get added to the menus in minimal profile. ... but I think the *test* is ok.
Comment #23
Gábor HojtsyDoes this work manually?
Comment #24
victor-shelepen CreditAttribution: victor-shelepen commentedManualy. It works.
Comment #25
YesCT CreditAttribution: YesCT commentedcleaned up some intermediate tries at testing.
some tests still needed.
unassigning, taking a break from this for a bit.
when trying manually, try it on a minimal profile.
also, run the test locally to see the verbose message screens.
Comment #26
xjmNice work! One thing to keep in mind is that the minimal profile and the testing profile are two different profiles.
A few suggestions:
This assertion is probably redundant now that we have the check for the translate link.
This assertion is the core of what we want to test -- looks good!
Once this is working (I think it is?), a good next step would be to create the menu link programmatically instead of with
drupalPost()
. (Similarly for the vocabulary.)So, it's bad practice to rely on default anything provided by the testing profile. We always want a decoupled test implementation.
In this case, this couples the test to both the Node module and the Frontpage view. See #1541298: Remove Node module dependency from Testing profile.
So, instead, I'd suggest putting this in a separate test that uses one of the Views' modules base classes and test views.
Comment #28
xjmThe test was stuck, so I cancelled and requeued it.
#25: test_all_lists-2004710-25.patch queued for re-testing.
Comment #28.0
xjmI've got time to work with this task. I am working with Kyiv Drupal initiative.
Comment #30
kfritscheWorking on this now.
Comment #31
kfritscheMoved the single test to
do*ListTest()
function to have a better structured test and not everything in this single function. Moved the views test to a new Test which extends the UITestBase.The test is now working, except the new View test. I'm currently trying to figure out how it works.
But 1., 3. and 4. from #26 should be fixed.
Still lots of other Tests are marked as todo.
Comment #32
kfritscheSet to needs review.
Comment #34
kfritscheView Test works now.
Minor cleanup with comments.
More tomorrow.
Comment #35
kfritscheAs always forgot status change...
Comment #36
YesCT CreditAttribution: YesCT commentedHere is a clean up on just that.
I suggest we commit this, and then add the rest of the tests later. This can let the testbot go with these good tests that we have while we add the rest.
Comment #37
Gábor HojtsyI agree that the test to land now would be useful. Thanks, committed! We may want to work on a followup or more followups to complete the coverage.
Comment #38
Gábor HojtsyRetitled for what was achieved. I think continuing in #2019831: people and roles page broken would be good, adding tests for roles to verify the before/after break/pass of that patch. I asked @kfritsche to work on that next.
Comment #39
kfritscheFollow up for the remaining tests: #2027587: Add tests for custom blocks, contact forms, formats, shortcut listings and settings pages.
Comment #40
YesCT CreditAttribution: YesCT commentednovice followup
#2028067: Clean up the added tests, by not calling procedural wrapper and fixing grammar
Comment #42
victor-shelepen CreditAttribution: victor-shelepen commentedI have problems wih the test. I am resolving this issue. https://drupal.org/node/1978916.
I receive a lot of test errors that are not related with my task. Could anybody make some specifications? Where should I move? I understand I need correct tests. But, there are many tests. I do not know where to start.
Comment #43
YesCT CreditAttribution: YesCT commented@likin Thanks for working on that issue.
I think it is not a related to this config_translation issue though, so re-closing this.
Comment #43.0
YesCT CreditAttribution: YesCT commentedadded related