Skip to content

Fix #6190: Add tests for ApplicationLifecycleLogger - #6491

Open
Aranya0811 wants to merge 8 commits into
oppia:developfrom
Aranya0811:add-tests-application-lifecycle-logger
Open

Aranya0811 wants to merge 8 commits into
oppia:developfrom
Aranya0811:add-tests-application-lifecycle-logger

Conversation

@Aranya0811

@Aranya0811 Aranya0811 commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #6190

Explaination

Adds tests for ApplicationLifecycleLogger, which didn't have its own tests after it was split out of ApplicationLifecycleObserver.

I created ApplicationLifecycleLoggerTest (and its test target in BUILD.bazel). It calls the logger's methods directly, without going through a real activity. The tests check that:

  • Opening the app logs APK size, storage usage and feature flags, and doesn't log CPU usage when performance metrics are turned off.
  • Bringing the app to the foreground changes the session ID only when the app was in the background for longer than the inactivity limit, logs the foreground event, and logs CPU usage only when performance metrics are turned on.
  • Sending the app to the background logs the background event, logs the correct time spent in the foreground, and logs CPU usage when performance metrics are turned on.
  • Calling the lifecycle methods in the wrong order (background before foreground, foreground twice, or background twice) throws an IllegalStateException.
  • Resuming an activity updates the current screen, logs startup latency only the first time, and logs memory usage every time.
  • Pausing an activity sets the current screen to the background screen, and the screen starts out unspecified.

I also removed the ApplicationLifecycleLogger.kt entry from scripts/assets/test_file_exemptions.textproto. That entry exempted the file from needing a test, and now that it has a test file, the Testfile Presence Check requires the exemption to be removed.

As part of the issue, I also updated ApplicationLifecycleObserverTest and removed two tests that were testing the logger rather than the observer, since the new logger tests now cover them:

  • testObserver_getCurrentScreen_verifyInitialValueIsUnspecified
  • testObserver_onAppInForeground_doesNotLogCpuUsage

The remaining observer tests were kept because they run through a real activity and therefore verify the interaction between the observer and logger.

I also removed the TODO(#6190): Add tests for this class. comment from ApplicationLifecycleLogger, since the tests now exist.

Testing performed locally:

  • ApplicationLifecycleLoggerTest — 20 tests passed
  • ApplicationLifecycleObserverTest — 19 tests passed

Essential Checklist

  • The PR title starts with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The explanation section above starts with "Fixes #bugnum: " (If this PR fixes part of an issue, use instead: "Fixes part of #bugnum: ...".)
  • Any changes to scripts/assets files have their rationale included in the PR explanation.
  • The PR follows the style guide.
  • The PR does not contain any unnecessary code changes from Android Studio (reference).
  • The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
  • The PR is assigned to the appropriate reviewers (reference).

Disclosure of LLM Usage

  • Did you use AI/LLMs when working on this PR? Yes
  • If yes, describe the extent AI was used: I used AI/LLMs as a helper throughout the development process. I used them to:
    • Understand the issue and break it into smaller steps.
    • Get a first draft of the tests, which I then checked against the actual ApplicationLifecycleLogger implementation, its record keeper, and the existing observer and CPU snapshotter tests.
    • Understand test failures. For example, a CPU usage test kept failing until I read the CpuPerformanceSnapshotter implementation and existing snapshotter tests and learned that it handles changes through a queue, so the test needed runCurrent() before moving time forward.
    • Identify which observer tests were duplicates of the new logger tests and which were integration tests that should remain.

I ran both test targets myself and made the final decisions on what to include.

@Aranya0811
Aranya0811 requested a review from a team as a code owner September 30, 2026 19:45
@github-actions

Copy link
Copy Markdown
Contributor

@Aranya0811 this PR is being marked as draft because the PR description must contain 'Fixes #' or 'Fixes part of #' for each issue the PR is changing, and each one on its own line with no other text.

@github-actions
github-actions Bot marked this pull request as draft September 30, 2026 20:00
@Aranya0811
Aranya0811 marked this pull request as ready for review September 30, 2026 20:12
@Aranya0811
Aranya0811 requested a review from a team as a code owner October 1, 2026 01:29
@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Results

Coverage Analysis: SKIP ⏭️

This PR did not introduce any changes to Kotlin source or test files.

To learn more, visit the Oppia Android Code Coverage wiki page

@Aranya0811

Copy link
Copy Markdown
Contributor Author

Greetings @Neer-rn @adhiamboperes
The two coverage checks are failing on this PR with an error in a file I didn't change (algebraic_expression_input_providers_kt). It looks like a CI cache problem, and my tests pass locally. Could you please re-run the coverage jobs?

@Neer-rn

Neer-rn commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

@Aranya0811 Please look at the PR description and make sure its following the PR template. Also assign it to someone if you think its ready for review.

@oppiabot

oppiabot Bot commented Oct 8, 2026

Copy link
Copy Markdown

Hi @Aranya0811, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you! If you're unsure how to reassign this PR to a reviewer, please make sure to review the wiki page that details the Guidance on submitting PRs.

@oppiabot oppiabot Bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Oct 8, 2026
@oppiabot oppiabot Bot removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Oct 11, 2026
@Aranya0811

Copy link
Copy Markdown
Contributor Author

Greetings @Neer-rn, thanks for the feedback! I've fixed the PR description to follow the template and pushed a new commit to re-run the checks.

This branch has not been deployed

No deployments
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.

[Feature Request]: Add tests for ApplicationLifecycleLogger

2 participants