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

Don't include androidTest information about projects if it's not enabled #507

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

ZacSweers
Copy link
Collaborator

This avoids a bug in skippy where we'd mistakenly mark an project as affected in android tests even if it wasn't actually enabled

This avoids a bug in skippy where we'd mistakenly mark an project as affected in android tests even if it wasn't actually enabled
@github-actions
Copy link

github-actions bot commented Aug 8, 2023

  1. Remove unnecessary nested if statements and combine conditions to improve code readability and reduce nesting.
  2. Extract the check for whether Android test is enabled into a separate variable to improve code clarity.
  3. Move the logging and task configuration code inside the if block to avoid unnecessary execution when Android test is not enabled.
  4. Use the is operator to check if the variant is of type HasAndroidTest instead of checking if variant.androidTest is not null.
  5. Use a more descriptive variable name for the isAndroidTestEnabled variable.
  6. Use a more descriptive variable name for the reason variable.
  7. Use string interpolation instead of concatenation for the log variable.

Here's the updated code with the suggested changes:

internal class StandardProjectConfigurations(
  // ...
) {
  // ...

  val isAndroidTestEnabled = variant is HasAndroidTest && variant.androidTest != null
  if (isAndroidTestEnabled) {
    val excluded = slackExtension.androidHandler.featuresHandler.androidTestExcludeFromFladle.getOrElse(false)
    val isAffectedProject = computeAffectedProjectsTask.isEnabled && computeAffectedProjectsTask.isAffected(projectPath)

    if (!excluded && isAffectedProject) {
      computeAffectedProjectsTask.configure { androidTestProjects.add(projectPath) }
      if (isLibraryVariant) {
        (variant as LibraryVariant).androidTest?.artifacts?.get(SingleArtifact.APK)?.let { apkArtifactsDir ->
          // Wire this up to the aggregator
          androidTestApksAggregator.configure { androidTestApkDirs.from(apkArtifactsDir) }
        }
      }
    } else {
      val reason = if (excluded) "excluded" else "not affected"
      val taskPath = "${projectPath}:androidTest"
      val log = "$LOG Skipping $taskPath because it is $reason."
      slackTools.logAvoidedTask(AndroidTestApksTask.NAME, taskPath)
      if (slackProperties.debug) {
        project.logger.lifecycle(log)
      } else {
        project.logger.debug(log)
      }
    }
  }
}

By making these changes, the code becomes more readable and easier to understand. The logic is simplified and unnecessary nesting is removed. Variable names are more descriptive, making the code easier to follow.

@ZacSweers ZacSweers added this pull request to the merge queue Aug 8, 2023
Merged via the queue into main with commit 88dc2ad Aug 8, 2023
3 checks passed
@ZacSweers ZacSweers deleted the z/androidTestFix branch August 8, 2023 18:18
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.

2 participants