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.

Sign up for GitHub

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

Jump to bottom

Fix taskcluster breakage from #6524 #6533

Merged
merged 2 commits into from
Dec 20, 2024
Merged

Fix taskcluster breakage from #6524 #6533

merged 2 commits into from
Dec 20, 2024

Conversation

bendk
Copy link
Contributor

@bendk bendk commented Dec 19, 2024 •
edited
Loading

Build a JAR file with libjnidispatch and libmegazord, targetted at Desktop arches so that devs can run unit tests with it. Export this as a configuration and Maven package. This allows android-components code to run the unit tests and also simplifies the gradle code for our own components.

Named this full-megazord-libsForTest, which I think is a bit more clear than full-megazord-forUnitTests.

Pull Request checklist

  • Breaking changes: This PR follows our breaking change policy
    • This PR follows the breaking change policy:
      • This PR has no breaking API changes, or
      • There are corresponding PRs for our consumer applications that resolve the breaking changes and have been approved
  • Quality: This PR builds and tests run cleanly
    • Note:
      • For changes that need extra cross-platform testing, consider adding [ci full] to the PR title.
      • If this pull request includes a breaking change, consider cutting a new release after merging.
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry in CHANGELOG.md or an explanation of why it does not need one
    • Any breaking changes to Swift or Kotlin binding APIs are noted explicitly
  • Dependencies: This PR follows our dependency management guidelines
    • Any new dependencies are accompanied by a summary of the due diligence applied in selecting them.

Branch builds: add [firefox-android: branch-name] to the PR title.

@bendk
Taskcluster fixes based on the recent issues
10b9ce5 
One reason we didn't catch this sooner we relied posting `/taskcluster
ci full` in a GH comment, but that didn't actually run all the tasks.

- Added a set of test parameters for the github issue comment
- Run all tasks for issue comments
@bendk bendk force-pushed the push-zwsooxxtmnxv branch from db468bf to 4942777 Compare December 19, 2024 16:25
@bendk
Copy link
Contributor Author

bendk commented Dec 19, 2024

/taskcluster ci full

@bendk bendk force-pushed the push-zwsooxxtmnxv branch from 4942777 to 8f9691d Compare December 20, 2024 13:42
@bendk
Copy link
Contributor Author

bendk commented Dec 20, 2024

/taskcluster ci full

@bendk bendk changed the title WIP: Fix taskcluster breakage from #6524 Fix taskcluster breakage from #6524 Dec 20, 2024
@bendk
Copy link
Contributor Author

bendk commented Dec 20, 2024 •
edited
Loading

I think this one is ready for review now.

  • (Note to self) Make sure to check that module-build-full-megazord doesn't have errors at the end of it's log.

@bendk bendk requested a review from ncalexan December 20, 2024 13:50
bendk
bendk commented Dec 20, 2024
View reviewed changes
build-scripts/component-common.gradle
dependsOn(processTestResTask)
}

testTask.dependsOn(copyNativeMegazord)
Copy link
Contributor Author

@bendk bendk Dec 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

To make things work on android-components we need to package a JAR file with the libraries. A nice side benefit of this is that we can just depend on it directly rather than have the weird copy task.

bendk
bendk commented Dec 20, 2024
View reviewed changes
.buildconfig-android.yml
@@ -127,7 +127,7 @@ projects:
publications:
- name: full-megazord
type: aar
- name: full-megazord-forUnitTests
- name: full-megazord-libsForTests
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I renamed it because I think it's a bit more precise/clear since:

  • We're not shipping the Megazord .kt sources, just libraries
  • We are shipping libjnidispatch, which is a not really part of the Megazord it's just a library needed for the tests.

I'm open to other names or keeping the old name. One reason to keep the old name is that we don't need to land code in android-components, but it feels like it's worth it to me.

ncalexan
ncalexan approved these changes Dec 20, 2024
View reviewed changes
Copy link
Member

@ncalexan ncalexan left a comment

Choose a reason for hiding this comment

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

Nothing worrying me here -- if it's good for you and automation, it's good for me.

megazords/full/android/build.gradle Outdated
// Android.
//
// For libmegazord, we copy the desktop libs from the
// (rust-android-gradle plugin](https://github.com/mozilla/rust-android-gradle), which is
Copy link
Member

Choose a reason for hiding this comment

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

This markdown is busted -- maybe

// ([rust-android-gradle plugin](...) ...)

megazords/full/android/build.gradle Outdated
// Native megazord library, this is the one compatible with the user's local machine. We use it
// to run unit tests.
// to run uniffi-bindgen against
Copy link
Member

Choose a reason for hiding this comment

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

nit: trailing period.

@bendk
Fix taskcluster breakage from mozilla#6524
33975fe 
Build a JAR file with libjnidispatch and libmegazord, targetted at
Desktop arches so that devs can run unit tests with it.  Export this as
a configuration and Maven package.  This allows android-components code
to run the unit tests and also simplifies the gradle code for our own
components.

Named this full-megazord-libsForTest, which I think is a bit more clear
than full-megazord-forUnitTests.
@bendk bendk force-pushed the push-zwsooxxtmnxv branch from 8f9691d to 33975fe Compare December 20, 2024 18:07
@bendk bendk added this pull request to the merge queue Dec 20, 2024
Merged via the queue into mozilla:main with commit 176f82f Dec 20, 2024
15 checks passed
@bendk bendk deleted the push-zwsooxxtmnxv branch December 20, 2024 19:06
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 21, 2024
@bendk
Bug 1938574 - Fix Unit tests that depend on the app-services megazord…
efaa2dd 
…, r=RyanVM

The new package name is going to be `full-megazord-libsForTest`
(mozilla/application-services#6533).

Differential Revision: https://phabricator.services.mozilla.com/D232720
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this pull request Dec 28, 2024
@bendk
Bug 1938574 - Fix Unit tests that depend on the app-services megazord…
bcabd28 
…, r=RyanVM

The new package name is going to be `full-megazord-libsForTest`
(mozilla/application-services#6533).

Differential Revision: https://phabricator.services.mozilla.com/D232720
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Jan 1, 2025
@marco-c
Bug 1938574 - Fix Unit tests that depend on the app-services megazord…
d504cb4 
…, r=RyanVM

The new package name is going to be `full-megazord-libsForTest`
(mozilla/application-services#6533).

Differential Revision: https://phabricator.services.mozilla.com/D232720

UltraBlame original commit: 8c693fe9f2ce5d5b3a11625224f09e01e110caad
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Jan 1, 2025
@marco-c
Bug 1938574 - Fix Unit tests that depend on the app-services megazord…
fc85c45 
…, r=RyanVM

The new package name is going to be `full-megazord-libsForTest`
(mozilla/application-services#6533).

Differential Revision: https://phabricator.services.mozilla.com/D232720

UltraBlame original commit: 8c693fe9f2ce5d5b3a11625224f09e01e110caad
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Jan 1, 2025
@marco-c
Bug 1938574 - Fix Unit tests that depend on the app-services megazord…
456cb32 
…, r=RyanVM

The new package name is going to be `full-megazord-libsForTest`
(mozilla/application-services#6533).

Differential Revision: https://phabricator.services.mozilla.com/D232720

UltraBlame original commit: 8c693fe9f2ce5d5b3a11625224f09e01e110caad
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this pull request Jan 2, 2025
@bendk
Bug 1938574 - Fix Unit tests that depend on the app-services megazord…
79175be 
…, r=RyanVM

The new package name is going to be `full-megazord-libsForTest`
(mozilla/application-services#6533).

Differential Revision: https://phabricator.services.mozilla.com/D232720
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ncalexan ncalexan ncalexan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bendk @ncalexan