test: Updating older cross-build smoke tests (iOS, Android) - #7487
test: Updating older cross-build smoke tests (iOS, Android)#7487thesuzerain wants to merge 17 commits into
Conversation
| MOZILLA_RUST_COMPONENTS_AS_PATH = "megazords/ios-rust/MozillaRustComponents.xcframework" | ||
|
|
||
|
|
||
| def replace_swift_package_artifact(ios_repo_path, as_repo_path): |
There was a problem hiding this comment.
I don't love using regex here but given how often this artifact file is updated (with hashes, etc) it doesn't really seem possible to use a fixture for it.
| # - xcode + xcodebuild + xcodetools setup and running (a successful build of the firefox-ios repository) | ||
| # Arguments: | ||
| # --action => Can be either `run-tests` (default) or `build-without-testing` | ||
| # --use-local-repo => Use a local firefox-ios repository instead (at the provided path). Exclusive with `remote-repo-url`. |
There was a problem hiding this comment.
Looks outdated? There is a --firefox-dir FIREFOX_DIR, but no --use-local-repo or --remote-repo-url
|
Was able to force an android regression in testing with this patch: diff --git a/components/ads-client/src/lib.rs b/components/ads-client/src/lib.rs
index 2088ee16c..1279423e9 100644
--- a/components/ads-client/src/lib.rs
+++ b/components/ads-client/src/lib.rs
@@ -72,10 +72,10 @@ impl MozAdsClient {
#[uniffi::method(default(options = None))]
pub fn record_impression(
&self,
- impression_url: String,
+ impression_url_foo: String,
options: Option<MozAdsCallbackOptions>,
) -> AdsClientApiResult<()> {
- let url = AdsClientUrl::parse(&impression_url).map_err(|e| {
+ let url = AdsClientUrl::parse(&impression_url_foo).map_err(|e| {
ComponentError::RecordImpression(CallbackRequestError::InvalidUrl(e).into())
})?;
let ohttp = options.map(|o| o.ohttp).unwrap_or(false);and running |
| And if so desired: | ||
|
|
||
| ```bash | ||
| ./mach run |
There was a problem hiding this comment.
I tried this strategy this morning and it's having a strange new error with link_nss_rustlib from I think recent commits- will update or delete based on trying it out, but wnted to flag this.
There was a problem hiding this comment.
I believe this is related to some breaking changes in some of the monorepo patches this morning (things out of sync, etc) and not related to this. Will wait for some patches and I'll ensure that these are all OK before merging.
mhammond
left a comment
There was a problem hiding this comment.
This all seems very elaborate and unlikely to be widely used, but you aren't forcing anyone to use this and if it scratches your itch I've no objection. I think "hnt" is very unclear though.
| # License, v. 2.0. If a copy of the MPL was not distributed with this | ||
| # file, You can obtain one at https://mozilla.org/MPL/2.0/. | ||
|
|
||
| # Purpose: Run Firefox fenix tests against this application-services working tree. |
There was a problem hiding this comment.
I have no idea what HNT stands for and google isn't helping. It looks like it's somehow related to desktop, but this all says Fenix.
| @@ -0,0 +1,88 @@ | |||
| # How to locally test application-services components on HNT / Desktop | |||
There was a problem hiding this comment.
ditto here, no idea what a "hnt desktop" is?
|
|
||
| **Current workflow (recommended):** | ||
|
|
||
| 1. Verify the local build of `A-S` is ready for a desktop build. |
There was a problem hiding this comment.
You could consider documenting the strategy of having your a-s changes on a branch and vendoring from that, avoiding symlinks etc. For many use-cases, the re-vendor is needed, so it isn't going to be wasted work but instead will be working on what needs to land anyway.
(IOW, using this process when you need to actually re-vendor anyway is a waste of time, might as well just work directly on the vendor)
| 2. Move the A-S components folder in M-C/Firefox to a temporary rename. | ||
| 3. Create a symlink between the A-S and M-C components folders. | ||
| 4. Generate uniffi bindings. | ||
| 5. Run and build. |
There was a problem hiding this comment.
Should probably include "./mach vendor rust" and "./mach cargo vet" - only necessary if there have been dependency changes, but that would block (or at least slow) landing.
This is a draft PR bringing in smoke tests scripts for building application-services against various targets (Android and iOS here, HNT to come, see below). It deprecates some older similar tests that are very out of date, and remakes them with up-to-date building methods (and more test flexibility).
I used existing helpers from the existing smoke tests where possible.
See: build instructions for iOS and build instructions for Fenix for context about the steps.
This currently makes no breaking changes. It is also thought that the deprecated smoke tests have long fallen out of use.
Remaining work:
We want to build against HNT if possible as well, but currently, HNT doesn’t have a system for building against a local a-s like the other two do. There are a couple possibilities I've explored (eg: redirecting cargo paths) but none that have been trivial. As it may require non-test-script changes, I would propose we do that in another PR.
There is also a good amount of iOS and Android tests that fail naturally (seemingly even in clean unrelated builds), and most possible regressions in a-s don’t cause a build or test failure, so it may be prudent in the future to make to extend iOS/Android tests to cover a-s behaviour specifically now that we have these scripts.
Pull Request checklist
[ci full]to the PR title.