Refactor MTE-1885 [v122] Move scripts and other files to firefox folder #17310

isabelrios · 2023-11-14T18:40:47Z

📜 Tickets

Jira ticket

Let's move more files into firefox-ios folder.

L10n tests and import scripts
Docs

Also let's rename the github actions mobile test eng own.

📝 Checklist

You have to check all boxes before merging

Filled in the above information (tickets numbers and description of your work)
Updated the PR name to follow our PR naming guidelines
Wrote unit tests and/or ensured the tests suite is passing
When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
If needed I updated documentation / comments for complex code and public methods

isabelrios · 2023-11-14T18:41:18Z

firefox-ios/l10n_comments.txt

Not sure if we can even remove this file... it has not been updated in two years

@Delphine do you know what this is used for?

No idea. @boek Do you remember this?

It shouldn't be removed, it's used for the WidgetIntent.strings we have. Example here. The name definitely could be better than l10n_comments thought haha. But if the filename is changed then we should change it too in the script

oh I see, thanks @lmarceau ... should I move it then or wait for another round of changes? Not sure I can create a PR in LocalizationTools repo or to handle that change there

I think I can create a PR in there. The strings for v120 are currently being exported, so if we merge the current PR (17310) after that's done to avoid breaking l10n flow would be ok I think (should be done by end of week for sure) 👍 I just created a task for myself to adjust the script in LocalizationTools once this PR is merged.

that sounds great :) thanks!

isabelrios · 2023-11-14T18:43:17Z

firefox-ios/import-strings.sh

@flodolo as commented on focus-ios PR, I'm also moving this script here. I have changed the path in the bootstrap file where it is executed. Locally I get the import working, but please let me know if I'm missing any other changes that could break the automation around this.

isabelrios · 2023-11-14T18:45:08Z

.github/workflows/firefox-ios-import-strings.yml

@flodolo I also changed the name for the focus github action and seems that this change is not affecting anything, but doublechecking

No risk, we have our own workflow, we only use scripts from this repo.

flodolo

We should be good, since we use LocalizationTools to do the actual extraction

flodolo · 2023-11-14T19:02:54Z

.github/workflows/firefox-ios-import-strings.yml

No risk, we have our own workflow, we only use scripts from this repo.

flodolo · 2023-11-14T19:03:26Z

firefox-ios/l10n_comments.txt

@Delphine do you know what this is used for?

isabelrios · 2023-11-15T09:30:44Z

.github/workflows/firefox-ios-glean-probe-scraper.yml

@relud is it fine if we rename this?

isabelrios · 2023-11-15T09:31:23Z

.github/workflows/firefox-ios-publish-docc.yml

@lmarceau checking with you if we can rename as author of the github action

Yes should be safe to rename 👍

isabelrios · 2023-11-15T09:32:07Z

.github/workflows/firefox-ios-stale.yml

@lmarceau checking with you if we can rename as author of the github action

Yes should be safe to rename 👍

isabelrios · 2023-11-15T09:32:50Z

.github/workflows/firefox-ios-update-nimbus-experiments.yml

@jeddai checking with you if we can rename without breaking anything as author of the github action

flodolo · 2023-11-15T09:40:22Z

I double-checked one of these comments, and we still have it in the XLIFF file. So, something is using this file, but I have no clue how based on #8050

flodolo · 2023-11-15T09:42:46Z

And that's because the file is used from code that lives elsewhere, and needs to be updated as well
https://github.com/mozilla-mobile/LocalizationTools/blob/main/Sources/LocalizationTools/tasks/ExportTask.swift#L91-L98

mobiletest-ci-bot · 2023-11-15T09:43:57Z

	Messages
📖	Project coverage: 35.02%
📖	Edited 478 files
📖	Created 0 files

Client.app: Coverage: 32.51

File	Coverage
NimbusOnboardingTestingConfigUtility.swift	100.0%	✅

Generated by 🚫 Danger Swift against 71cf099

isabelrios · 2023-11-15T09:46:41Z

And that's because the file is used from code that lives elsewhere, and needs to be updated as well https://github.com/mozilla-mobile/LocalizationTools/blob/main/Sources/LocalizationTools/tasks/ExportTask.swift#L91-L98

Thanks for checking, I will not remove the l10n_comments.txt file then

mergify · 2023-11-16T18:05:45Z

This pull request has conflicts when rebasing. Could you fix it @isabelrios? 🙏

isabelrios · 2023-11-16T19:51:34Z

I had to create a firefox-ios-tests folder inside firefox-ios and move the tests there. The way I created firefox-ios folder before would not allow me to move app folders in it in the future.

This PR is ready for review. Thanks for your comments so far.

isabelrios · 2023-11-17T08:30:57Z

@flodolo I'm moving the import-strings.sh script into firefor-ios folder. I have changed the bootstrap script to run it using the new location. Are there more changes needed so that the import/export keeps working? The import is working for me locally, but double checking as I don't want to break things

flodolo · 2023-11-17T08:38:08Z

@flodolo I'm moving the import-strings.sh script into firefor-ios folder. I have changed the bootstrap script to run it using the new location. Are there more changes needed so that the import/export keeps working? The import is working for me locally, but double checking as I don't want to break things

We only use the bootstrap from this repo and LocalizationTools on our side. Once this is merged, you should verify that the import action works correctly in this repo.

isabelrios · 2023-11-17T08:41:19Z

@flodolo I'm moving the import-strings.sh script into firefor-ios folder. I have changed the bootstrap script to run it using the new location. Are there more changes needed so that the import/export keeps working? The import is working for me locally, but double checking as I don't want to break things

We only use the bootstrap from this repo and LocalizationTools on our side. Once this is merged, you should verify that the import action works correctly in this repo.

I will verify that for sure, thank you

isabelrios · 2023-11-17T10:50:21Z

FYi, This PR will not land until the v121 branch is cut from main on Monday to disrupt the less possible

mergify · 2023-11-17T13:53:51Z

This pull request has conflicts when rebasing. Could you fix it @isabelrios? 🙏

clarmso · 2023-11-17T14:23:31Z

😬 Would Sync Integration Tests run? I can give it a try.

isabelrios · 2023-11-17T14:25:28Z

😬 Would Sync Integration Tests run? I can give it a try.

In theory yes, on MacStadium I will fix the path to the test results once this lands

clarmso · 2023-11-17T15:21:29Z

😬 Would Sync Integration Tests run? I can give it a try.

In theory yes, on MacStadium I will fix the path to the test results once this lands

🎉 I just confirmed that the SyncIntegrationTests could be run.

clarmso · 2023-11-17T15:43:32Z

😬 Would Sync Integration Tests run? I can give it a try.

In theory yes, on MacStadium I will fix the path to the test results once this lands

🎉 I just confirmed that the SyncIntegrationTests could be run.

I take it back. The TPS logs shows the following errors. @isabelrios Could you please update the paths to files needed by TPS?

2023-11-17 10:24:40.966 CROSSWEAVE ERROR: [phase -1] _executeTestPhase failed - Error opening input stream (invalid filename?): file:///Users/cso/workspace/firefox-ios/test_bookmark_desktop.js No traceback available

clarmso

Please check paths for sync integration tests. 🙇🏼

isabelrios · 2023-11-17T16:51:09Z

😬 Would Sync Integration Tests run? I can give it a try.

In theory yes, on MacStadium I will fix the path to the test results once this lands

🎉 I just confirmed that the SyncIntegrationTests could be run.

I take it back. The TPS logs shows the following errors. @isabelrios Could you please update the paths to files needed by TPS?
2023-11-17 10:24:40.966 CROSSWEAVE ERROR: [phase -1] _executeTestPhase failed - Error opening input stream (invalid filename?): file:///Users/cso/workspace/firefox-ios/test_bookmark_desktop.js No traceback available

@clarmso I have a copy of my branch on MacStadium and the tests passed there. It may be something on your virtual env that is chached?
You can check on MacStadium: /Users/administrator/irios-tmp-test/irios-fork/firefox-ios/firefox-ios/firefox-ios-tests/Tests/SyncIntegrationTests
I have left the terminal open with my repo and branch. I commented the test_integration file to just run one test to try

clarmso · 2023-11-17T19:07:12Z

😬 Would Sync Integration Tests run? I can give it a try.

In theory yes, on MacStadium I will fix the path to the test results once this lands

🎉 I just confirmed that the SyncIntegrationTests could be run.

I take it back. The TPS logs shows the following errors. @isabelrios Could you please update the paths to files needed by TPS?
2023-11-17 10:24:40.966 CROSSWEAVE ERROR: [phase -1] _executeTestPhase failed - Error opening input stream (invalid filename?): file:///Users/cso/workspace/firefox-ios/test_bookmark_desktop.js No traceback available
@clarmso I have a copy of my branch on MacStadium and the tests passed there. It may be something on your virtual env that is chached? You can check on MacStadium: /Users/administrator/irios-tmp-test/irios-fork/firefox-ios/firefox-ios/firefox-ios-tests/Tests/SyncIntegrationTests I have left the terminal open with my repo and branch. I commented the test_integration file to just run one test to try

😓 pipenv clean is needed in the directory after checking out the branch.

mergify · 2023-11-20T10:19:03Z

This pull request has conflicts when rebasing. Could you fix it @isabelrios? 🙏

mergify · 2023-11-20T15:18:28Z

This pull request has conflicts when rebasing. Could you fix it @isabelrios? 🙏

mergify · 2023-11-20T23:15:24Z

This pull request has conflicts when rebasing. Could you fix it @isabelrios? 🙏

isabelrios commented Nov 14, 2023

View reviewed changes

flodolo reviewed Nov 14, 2023

View reviewed changes

isabelrios commented Nov 15, 2023

View reviewed changes

isabelrios marked this pull request as ready for review November 15, 2023 09:33

isabelrios requested review from a team as code owners November 15, 2023 09:33

isabelrios requested a review from jjSDET November 15, 2023 09:33

isabelrios force-pushed the irios-mte1885-move-test-scripts-files-into-firefoxios branch from 743d77b to 4f9b3d7 Compare November 16, 2023 18:12

isabelrios requested review from OrlaM, lmarceau and clarmso November 16, 2023 19:49

lmarceau approved these changes Nov 16, 2023

View reviewed changes

clarmso approved these changes Nov 17, 2023

View reviewed changes

clarmso requested changes Nov 17, 2023

View reviewed changes

isabelrios requested a review from clarmso November 17, 2023 16:51

clarmso approved these changes Nov 17, 2023

View reviewed changes

OrlaM approved these changes Nov 17, 2023

View reviewed changes

isabelrios force-pushed the irios-mte1885-move-test-scripts-files-into-firefoxios branch from 6125091 to 55afc47 Compare November 20, 2023 09:37

isabelrios force-pushed the irios-mte1885-move-test-scripts-files-into-firefoxios branch from 55afc47 to c5ebd8e Compare November 20, 2023 10:52

isabelrios force-pushed the irios-mte1885-move-test-scripts-files-into-firefoxios branch from c5ebd8e to 65abf87 Compare November 20, 2023 16:04

isabelrios added 5 commits November 21, 2023 09:32

Refactor MTE-1885 Move scripts and other files to firefox folder

03e54c4

rename all github actions

3d4bfa4

create firefox-ios folder and move tests there

63db7dc

new path for swiftlint and fix sync int tests path

eec1398

fix path for swiftlint

023fafa

isabelrios force-pushed the irios-mte1885-move-test-scripts-files-into-firefoxios branch from 65abf87 to 023fafa Compare November 21, 2023 08:32

isabelrios added 3 commits November 21, 2023 11:30

remove test suite that was removed in previous commit

164d721

remove test suite that was removed in previous commit2

57db256

remove unchanged files

71cf099

isabelrios merged commit 48a02a1 into mozilla-mobile:main Nov 21, 2023
10 checks passed

isabelrios changed the title ~~Refactor MTE-1885 Move scripts and other files to firefox folder~~ Refactor MTE-1885 [v122] Move scripts and other files to firefox folder Nov 23, 2023

Refactor MTE-1885 [v122] Move scripts and other files to firefox folder #17310

Refactor MTE-1885 [v122] Move scripts and other files to firefox folder #17310

Conversation

isabelrios commented Nov 14, 2023

📜 Tickets

📝 Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flodolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flodolo commented Nov 15, 2023

flodolo commented Nov 15, 2023

mobiletest-ci-bot commented Nov 15, 2023 • edited Loading

Client.app: Coverage: 32.51

isabelrios commented Nov 15, 2023

mergify bot commented Nov 16, 2023

isabelrios commented Nov 16, 2023

isabelrios commented Nov 17, 2023

flodolo commented Nov 17, 2023

isabelrios commented Nov 17, 2023

isabelrios commented Nov 17, 2023

mergify bot commented Nov 17, 2023

clarmso commented Nov 17, 2023

isabelrios commented Nov 17, 2023

clarmso commented Nov 17, 2023

clarmso commented Nov 17, 2023

clarmso left a comment

Choose a reason for hiding this comment

isabelrios commented Nov 17, 2023 • edited Loading

clarmso commented Nov 17, 2023

mergify bot commented Nov 20, 2023

mergify bot commented Nov 20, 2023

mergify bot commented Nov 20, 2023

mobiletest-ci-bot commented Nov 15, 2023 •

edited

Loading

isabelrios commented Nov 17, 2023 •

edited

Loading