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

Avoid eager initialization of bugsnag tasks #266

Merged
merged 23 commits into from
Aug 17, 2020
Merged

Conversation

fractalwrench
Copy link
Contributor

@fractalwrench fractalwrench commented Aug 14, 2020

Goal

Avoids eager initialization of bugsnag tasks by supporting Task Configuration Avoidance. Previous changesets such as #227 made pre-requisite changes, but the use of task.get() triggered creation of bugsnag tasks during the Configuration phase which impacts build performance.

Changeset

This changeset builds on #261 which has been reviewed by @fractalwrench. The majority of the new changes in this PR relate to the E2E scenarios as the request order has changed due to the different mechanism used to register bugsnag tasks.

BugsnagUploadProguardTask has been updated to use a ConfigurableFileCollection and @InputFiles for the mapping file property. These do not require a file to exist at the time of task registration - which would otherwise fail the build as the mapping file doesn't exist until late in the build lifecycle. This mirrors the approach already used for BugsnagReleasesTask.

On AGP 3.5.X the tasks add a mustRunAfter dependency on the package task, as the ordering is otherwise incorrect and the mapping file would not exist. This leads to a degradation as configuration avoidance can't be supported, and also results in manual invocations of the upload/bundle upload tasks failing. This is seen as acceptable due to the low number of users on this version, the less common use-case, and a simple workaround of updating AGP version.

Tests

  • Updated E2E scenarios to deal with new request order
  • Skipped manual upload scenarios on AGP 3.5.0 for graceful degradation of behaviour
  • Ran a gradle build scan and confirmed that only 2 tasks are created by the bugsnag plugin during the configuration phase, whereas previously 220 were created

Copy link
Contributor

@ZacSweers ZacSweers left a comment

Choose a reason for hiding this comment

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

Nice!

@fractalwrench fractalwrench marked this pull request as ready for review August 17, 2020 08:04
Copy link
Member

@imjoehaines imjoehaines left a comment

Choose a reason for hiding this comment

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

Do we have a ticket to fix & re-enable the newly skipped tests? I'm a bit concerned by the loss of coverage, but otherwise this looks good

features/support/env.rb Outdated Show resolved Hide resolved
Co-authored-by: Joe Haines <hello@joehaines.co.uk>
@fractalwrench
Copy link
Contributor Author

PLAT-4842 should cover allowing the tests to be enabled once more - this is just an issue with the order in which requests are received on particular AGP versions.

@fractalwrench fractalwrench merged commit dcc78e9 into v5 Aug 17, 2020
@fractalwrench fractalwrench deleted the v5-config-avoidance branch August 17, 2020 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants