Problem/Motivation

Found while working on #2226533: Changes to the Language class due to the LanguageInterface (followup)

Making the id property protected, so... need to not change it in a language like this.

Proposed resolution

Make a new language and use constructor to set the id, or
Maybe we just dont use this change it was thinking it needed to make.

Remaining tasks

see what happens with initial patch
get feedback

User interface changes

No

API changes

No

CommentFileSizeAuthor
#7 2333907.7.patch4.11 KBalexpott
#4 2333907.2.patch1.53 KBYesCT
#2 2332739.2.patch4.84 KBYesCT
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

YesCT’s picture

Issue summary: View changes
Status: Active » Needs review

patch to explain my idea.

YesCT’s picture

FileSize
4.84 KB

uh. and the patch.

Status: Needs review » Needs work

The last submitted patch, 2: 2332739.2.patch, failed testing.

YesCT’s picture

FileSize
1.53 KB

crap. right patch.

YesCT’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 4: 2333907.2.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
FileSize
4.11 KB

We can completely remove the user part of the test - it's not really doing much apart from making things complicated - language negotiation, the ability of user's to set timezones and preferred languages are all tested elsewhere.

YesCT’s picture

but I wonder if this is testing to see if things are showing in the user preferred language.
Where is "elsewhere"?

I'll look again.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +sprint

This test definitely does not test setting a user preferred language, because even though it sets the user to prefer a language, it does not set the negotiation to include that condition, so it needs to manually fiddle with the interface language to be equal to the user preference, which is the point of this issue... So I agree it just makes the test too complex. UserTimeZoneTest looks like a good test for user timezones that does what this part of this test did. So looks good to me.

YesCT’s picture

ok. :)

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0,x, thanks!

  • catch committed 462ba73 on 8.0.x
    Issue #2333907 by YesCT, alexpott: Fixed FormatDateTest incorrectly...
Gábor Hojtsy’s picture

Issue tags: -sprint

Yay, thanks.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.