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

[Tests-Only] Add acceptance test for translated response messages #37069

Merged
merged 1 commit into from
Mar 5, 2020

Conversation

jasson99
Copy link
Contributor

@jasson99 jasson99 commented Mar 4, 2020

Description

This PR adds acceptance test for the translated error messages when the Accept-Language header is set to some preferred language.

Related Issue

owncloud/QA#608

How Has This Been Tested?

CI

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

dpakach
dpakach previously approved these changes Mar 4, 2020
Copy link
Contributor

@dpakach dpakach left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jasson99 jasson99 dismissed dpakach’s stale review March 4, 2020 09:30

some reviews to fix

@codecov
Copy link

codecov bot commented Mar 4, 2020

Codecov Report

Merging #37069 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #37069   +/-   ##
=========================================
  Coverage     64.75%   64.76%           
- Complexity    19135    19136    +1     
=========================================
  Files          1270     1270           
  Lines         74909    74915    +6     
  Branches       1329     1328    -1     
=========================================
+ Hits          48511    48517    +6     
- Misses        26007    26008    +1     
+ Partials        391      390    -1     
Flag Coverage Δ Complexity Δ
#javascript 54.18% <ø> (+0.01%) 0.00 <ø> (ø)
#phpunit 65.93% <ø> (+<0.01%) 19136.00 <ø> (+1.00)
Impacted Files Coverage Δ Complexity Δ
apps/files_external/lib/Lib/Storage/Google.php 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
core/js/shareitemmodel.js 80.51% <0.00%> (+0.41%) 0.00% <0.00%> (ø%)
apps/files/lib/Command/TransferOwnership.php 81.57% <0.00%> (+0.75%) 43.00% <0.00%> (+1.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90bb5e9...0325710. Read the comment docs.

@phil-davis
Copy link
Contributor

@dpakach please review again. This looks OK - I guess the risk is the a translator of one of these languages decides to change the translation, and then CI will suddenly break. But it will be easy to fix if it happens.

Then the commits can be squashed before merging.

Copy link
Contributor

@dpakach dpakach left a comment

Choose a reason for hiding this comment

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

Use outline to test for different languages rather than repeating it every time.

@jasson99 jasson99 requested a review from dpakach March 5, 2020 04:04
Copy link
Contributor

@dpakach dpakach left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

For this sort of scenario it is OK to have repeated When/Then - otherwise it takes so much time if we have 8 rows in the examples table...

@phil-davis
Copy link
Contributor

drone had timeouts pulling some docker images. I restarted it.

@jasson99 jasson99 merged commit d64902c into master Mar 5, 2020
@delete-merged-branch delete-merged-branch bot deleted the translation branch March 5, 2020 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants