Skip to content

Conversation

@mattcreaser
Copy link
Member

  • PR title and description conform to Pull Request guidelines.

Issue #, if available:

Description of changes:

This PR attempts to resolve multiple issues to make the Auth integration tests more stable, or at least make it easier to understand the cause of failures.

The Auth integration tests had mostly been implemented with a sequence of operations that looked like this:

val latch = CountdownLatch(1)
Amplify.Auth.doSomething(
    { 
        assertEquals(result, expectedResult)
        latch.countDown()
    },
    { fail("Error occurred") }
)
latch.await(TIMEOUT, TimeUnit.SECONDS)

or sometimes

val latch = CountdownLatch(1)
Amplify.Auth.doSomething(
    { 
        assertEquals(result, expectedResult)
        latch.countDown()
    },
    { latch.countDown() }
)
latch.await(TIMEOUT, TimeUnit.SECONDS)

This style of testing has a variety of issues:

  1. Latch.await result not checked. Most tests did not assert that latch.await returned true. If a CountdownLatch times out while waiting it returns false, and by not checking for this value tests could continue executing while the previous operation was still in progress.
  2. Assertions on background threads. The callbacks from Amplify APIs happen on background threads, while assertions in JUnit tests must happen on the test thread. So any calls to assertEquals or fail in a callback to an Amplify API will not fail the test as desired. What actually happens is timing dependent but usually the test process will crash, which may happen in the middle of an execution of a different test, resulting in confusing failures.
  3. Errors not handled properly. In the first example above the actual returned error is discarded in favour of a generic failure message. In the second example the error is just completely ignored, likely resulting in incorrect test behaviour. If these tests fail it's hard to understand why from the information returned by Device Farm.

This PR aims to resolve these issues in the following ways:

  1. Replace many calls with calls to SynchronousAuth and SynchronousApi. These test utilities already resolve many of these issues by returning errors to the main thread before throwing them and internally ensuring timeouts are handled.
  2. Added additional test utilities for avoiding these pitfalls such as CountdownLatch.assertAwait and particularly around the handling of API subscriptions for receiving OTP codes.
  3. Improved error messaging for test failures.

How did you test these changes?
Verified tests run successfully when run on my local device.

Documentation update required?

  • No
  • Yes (Please include a PR link for the documentation update)

General Checklist

  • Added Unit Tests
  • Added Integration Tests
  • Security oriented best practices and standards are followed (e.g. using input sanitization, principle of least privilege, etc)
  • Ensure commit message has the appropriate scope (e.g fix(storage): message, feat(auth): message, chore(all): message)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mattcreaser mattcreaser requested a review from a team as a code owner October 22, 2025 14:51
@codecov
Copy link

codecov bot commented Oct 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.31%. Comparing base (ef37f18) to head (fb24b3b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3143      +/-   ##
==========================================
- Coverage   54.32%   54.31%   -0.02%     
==========================================
  Files        1041     1041              
  Lines       31926    31926              
  Branches     4708     4708              
==========================================
- Hits        17344    17340       -4     
- Misses      12746    12748       +2     
- Partials     1836     1838       +2     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@tylerjroach tylerjroach left a comment

Choose a reason for hiding this comment

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

This is a lot cleaner!

@mattcreaser mattcreaser merged commit cade307 into main Oct 22, 2025
17 of 18 checks passed
@mattcreaser mattcreaser deleted the mattcreaser/auth-integration-tests-improvements branch October 22, 2025 19:04
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