Skip to content

Add Comprehensive Appium E2E Testing Framework for Sefaria Mobile App#175

Open
HershelT wants to merge 70 commits into
masterfrom
automatic-e2e-tests-integration
Open

Add Comprehensive Appium E2E Testing Framework for Sefaria Mobile App#175
HershelT wants to merge 70 commits into
masterfrom
automatic-e2e-tests-integration

Conversation

@HershelT

Copy link
Copy Markdown
Collaborator

This PR introduces a robust, modular end-to-end (E2E) testing framework for the Sefaria mobile app, built with WebdriverIO and Appium. The framework enables automated testing on Android devices both locally and via BrowserStack, supporting continuous integration with GitHub Actions.

Key Features:

Modular test structure for easy maintenance and expansion
Automated logging of test runs and UI errors (with screenshots)
Support for local and cloud (BrowserStack) device testing
Clear documentation for setup, running, and contributing new tests
Utilities for interacting with app UI, navigation, and Sefaria APIs
Example tests covering navigation, language toggling, learning schedules, and more
Why this matters:
Automated E2E testing will help ensure the quality and reliability of the Sefaria app across updates, reduce manual QA effort, and make it easier for contributors to add new tests as features evolve.

How to get started:
See the included README.md for setup instructions, prerequisites, and contribution guidelines. All contributors are welcome to add new tests or improve existing ones!

@HershelT

HershelT commented Jul 14, 2025

Copy link
Copy Markdown
Collaborator Author

@akiva10b @yitzhakc @schreibersefaria Pull Request created. Woohoo Automatic tests!

HershelT added 3 commits July 16, 2025 11:52
…ions, and creating texts tab check (almost done)
…gewt the topics page and specific topics, enhanced ui checker for specific screenshot checking, and continued to use newly implemented text_constants

@schreibersefaria schreibersefaria left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It would be helpful if you fix up the readme before I continue the PR.
Alternatively, could you add to the PR description more details about each file and the general structure.

Comment thread automatic-e2e-tests/android/testing-framework/components/topics_page.ts Outdated
Comment thread automatic-e2e-tests/ReadMe.md Outdated
HershelT added 4 commits July 17, 2025 10:17
Seperated the README into 4 seperate components for better instructions and clarity. (SETUP, TEST_GUIDE, FILE_OVERVIEW, README). This creates greater understanding of what everything does, how to create tests, and easier way to run them.
Also, added headers to each file, explaining what each file does, its purpose, and its usage.
…f concerns. FILE_OVERVIEW reflects this change

@schreibersefaria schreibersefaria left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm still (almost) only commenting on the MD files but enough has accumulated to push the review.
I think it would make sense to huddle to discuss the PR before you put too much work into it and before I continue to the rest of the content

Comment thread automatic-e2e-tests/ReadMe.md Outdated
Comment thread automatic-e2e-tests/FILE_OVERVIEW.md Outdated
Comment thread automatic-e2e-tests/ReadMe.md Outdated
Comment thread automatic-e2e-tests/ReadMe.md Outdated
Comment thread automatic-e2e-tests/SETUP.md Outdated
Comment thread automatic-e2e-tests/TEST_GUIDE.md Outdated
Comment thread automatic-e2e-tests/TEST_GUIDE.md Outdated
Comment thread automatic-e2e-tests/TEST_GUIDE.md Outdated
Comment thread automatic-e2e-tests/TEST_GUIDE.md
Comment thread automatic-e2e-tests/TEST_GUIDE.md
HershelT added 6 commits July 21, 2025 14:56
…xed browserstack error where it would not launch app.
…ainability

- Introduced centralized constants for gestures, colors, timeouts, and selectors to replace hardcoded values across the testing framework.
- Updated gesture utility functions to utilize new gesture configuration constants for swipe distances and durations.
- Refactored UI interaction utilities to use centralized selectors for offline popups and text finding, enhancing consistency.
- Enhanced timeout management by consolidating timeout values into a dedicated constants file for better readability and maintenance.
- Improved documentation across various utility files to clarify usage and structure.
- Consolidated static and dynamic error messages into structured objects in error_constants.ts for better organization and maintainability.
- Updated all references to error messages throughout the codebase to use the new structure.
- Removed unused gesture validation thresholds from gestures.ts.
- Cleaned up selectors.ts by adding a new selector for content description.
- Removed unnecessary timeout constants from timeouts.ts.
- Enhanced logging in e2e tests to provide clearer information on test execution status.
- Updated helper functions and UI checker utilities to utilize the new error handling structure.
- Improved debug logging for better traceability during test execution.
… and maintainability across multiple components

@schreibersefaria schreibersefaria left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hope to do some more tomorrow

Comment thread automatic-e2e-tests/FILE_OVERVIEW.md
Comment thread automatic-e2e-tests/FILE_OVERVIEW.md Outdated
Comment thread automatic-e2e-tests/FILE_OVERVIEW.md Outdated
Comment thread automatic-e2e-tests/FILE_OVERVIEW.md Outdated
Central import point for all components, allowing easy access to all page objects.

- **\*.ts**
Add new component helpers as needed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ahh I think it's understood implicitly

Comment thread automatic-e2e-tests/SETUP.md Outdated
- Upload your APK/AAB (Android) or IPA (iOS) file to BrowserStack using their [App Upload page](https://app-automate.browserstack.com/dashboard/v2/app-upload).
- Copy the App ID (e.g., `bs://<app-id>`) and set it in your `.env` as `ANDROID_BROWSERSTACK_APP_ID` or `IOS_BROWSERSTACK_APP_ID`:
```env
- **Note:** You must upload your app every time you build a new APK/AAB/IPA. The App ID changes with each upload, so always update `*_BROWSERSTACK_APP_ID` in your `.env`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

mmm in that case personally I would prefer not to have it at all.
I'd probable just run it locally anyway. I'm not sure I even have permissions to edit the secrets

Comment thread automatic-e2e-tests/shared/tests/e2e.spec.ts Outdated
Comment thread automatic-e2e-tests/shared/tests/e2e.spec.ts Outdated
if (parasha) {
await TEXT_FINDER.findTextElement(client, parasha.displayValue.en);
} else {
throw new Error(DYNAMIC_ERRORS.apiResultMismatch("Parashat Hashavua", parasha!.displayValue.en));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

but this if is on SEFARIA_API.getCurrentParashatHashavua not TEXT_FINDER

Comment thread automatic-e2e-tests/tests/e2e.spec.ts Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not sure where the conversation about this is so opening it again.
I think it makes more sense to have the tests in logical files.
You would then define what tests are regression somewhere else.

@HershelT HershelT Aug 13, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hmmm, logically that makes sense. I will have to go through and possible edit my parallel running script if I do that.

Comment thread automatic-e2e-tests/utils/text_finder.ts

@schreibersefaria schreibersefaria left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I've added some comments that generalise to most the code.
The big things I noticed are error handling and return values.
In general, consistency is key (but very hard to do).

Comment thread automatic-e2e-tests/components/display_settings.ts
Comment thread automatic-e2e-tests/components/display_settings.ts Outdated
Comment thread automatic-e2e-tests/components/display_settings.ts
Comment thread automatic-e2e-tests/components/display_settings.ts Outdated
* @param client - WebdriverIO browser instance.
* @param isEnglish - Current language state; true if English, false if Hebrew.
*/
export async function toggleLanguageButton(client: Browser, isEnglish: boolean = true): Promise<void> {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't think you need isEnglish, you can just check the current language.
It could be more elegant to have a param of targetLanguage, then you check if we are in the target language and only change if needed.

Comment thread automatic-e2e-tests/components/display_settings.ts Outdated
Comment thread automatic-e2e-tests/components/display_settings.ts
Comment thread automatic-e2e-tests/components/reader_page.ts
Comment thread automatic-e2e-tests/components/reader_page.ts
Comment thread automatic-e2e-tests/components/reader_page.ts Outdated
…larity. Turn Uppercase into PascalCase, and working on continuing to make imports better and more readable
…dication and improve error handling with helper function HelperFunctions.ensureElementDisplayed().
@HershelT HershelT requested a review from Copilot August 31, 2025 13:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a comprehensive end-to-end testing framework for the Sefaria mobile app using WebdriverIO and Appium, supporting both Android and iOS platforms with local and cloud testing capabilities.

  • Modular, cross-platform architecture with centralized constants and platform-specific selectors
  • Automated UI validation including color pixel testing and visual regression capabilities
  • Integration with Sefaria APIs for dynamic content validation and BrowserStack for cloud testing

Reviewed Changes

Copilot reviewed 39 out of 40 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
utils/ui_checker.ts Pixel color validation utilities with cross-platform coordinate system handling
utils/text_finder.ts Text and element location helpers for robust UI interaction
utils/sefaria_api.ts API utilities for fetching calendar data and learning schedules
utils/offline_popup.ts Popup handling utilities for app initialization flows
utils/load_credentials.ts Platform-agnostic credential loader for local and BrowserStack configurations
utils/helper_functions.ts Core helper functions for text matching, color conversion, and test lifecycle
utils/gesture.ts Gesture and scrolling utilities with platform-aware element visibility
utils/browserstack_report.ts BrowserStack session reporting and status management
constants/*.ts Centralized configuration for timeouts, colors, gestures, and error messages
components/*.ts Page object models for navbar, reader, search, topics, and display settings
selectors/*.ts Platform-specific UI selectors for Android and iOS
package.json Dependencies and test execution scripts
Other files Configuration, documentation, and test execution infrastructure
Comments suppressed due to low confidence (1)

automatic-e2e-tests/constants/errors.ts:1

  • Using non-null assertion parasha!.displayValue.en when parasha was already checked to be falsy in the condition above. This will throw a runtime error. The error message should use a default value or handle the undefined case properly.
/**

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread automatic-e2e-tests/utils/ui_checker.ts Outdated
Comment thread automatic-e2e-tests/utils/sefaria_api.ts Outdated
Comment thread automatic-e2e-tests/utils/sefaria_api.ts Outdated
Comment thread automatic-e2e-tests/utils/helper_functions.ts Outdated
Comment thread automatic-e2e-tests/utils/gesture.ts
Comment thread automatic-e2e-tests/tests/e2e.spec.ts Outdated
Comment thread automatic-e2e-tests/tests/e2e.spec.ts Outdated

@schreibersefaria schreibersefaria left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A few big comments and some smaller ones.
Overall it looks to be going in the right direction but there are some big changes still ahead of us.
As you may notice I haven't yet gone over every line so after you get to all the comments, please request another review and ping me.
And you don't have to wait for the next review for questions. I'm generally available :-)

Comment thread automatic-e2e-tests/FILE_OVERVIEW.md Outdated
Centralized logs and screenshots for both platforms.

- **scripts/**
Utility scripts (e.g., cleanup.js). `run-parallel-tests.js` for running tests in parallel on multiple devices.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You give two examples. But one is in brackets and one isn't.
I'd would unify the format here

Comment thread automatic-e2e-tests/FILE_OVERVIEW.md Outdated

---

## components/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Confusing styling.
Some dirs are h2 and some are bold.
Also, I'd order the dirs by importance

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm not sure why this is needed.
I understood the logic around selectors but not sure it duplicates to other dirs.

Selectors = require('../selectors/android/selectors');
}

export { Selectors };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I find it confusing that you combined the index of constants and selectors.
Was there a reason for that

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here too, if there isn't a unified ios/android import, I don't know if there is a need for the index file.
If people need to import a lot of constance then a long import line / many imports are appropriate

export const APP_PACKAGE = 'org.sefaria.sefaria';

// Base UiSelector patterns
export const BASE_SELECTORS = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Another naming comment.
Not sure why selectors would be all caps. They are functions.
This could be a best practice I'm not familiar with


// iOS Accessibility patterns
export const ACCESSIBILITY_PATTERNS = {
englishTextPrefix: '⁦',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If you are using a hidden unicode char you should definitely comment it

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Generally in both selector files, some more commenting would be very helpful for anyone who needs to edit it

* @throws Will throw an error if the text is not found
* @returns boolean indicating success
*/
export async function verifyTitleContains(client: Browser, expectedText: string): Promise<boolean> {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A lot of duplication from the previous function.
Could you combine them?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You have one document for all topic related things.
Maybe split each page within topics (the actual topic page and the navigation pages) into different files

@yitzhakc yitzhakc removed their request for review October 16, 2025 11:39
HershelT added 3 commits June 1, 2026 09:40
- Add CLAUDE.md: architecture, env vars, session-per-suite lifecycle,
  conventions, and platform gotchas
- Rename ReadMe.md -> README.md so cross-doc backlinks resolve; rewrite
  with an accurate <suite>:<platform>:<env> command reference (documents
  regression:* and the test:* vs regression/sanity distinction)
- SETUP.md: fix malformed/unclosed env code fence; tabulate env config,
  troubleshooting, and CI secrets
- TEST_GUIDE.md: correct to session-per-suite model and ../log_init import;
  fix handleSetup/handleTeardown casing and duplicate PopUps import
- FILE_OVERVIEW.md: fix wrong filenames (gesture_constants/popups/sefaria_api)
  and add missing files (log_init, .mocharc*, tsconfig, scripts)
- example.env: fix swapped iOS/Android example app-path comments
- Delete the unused `isModularization` env-flag (read but never used)
- Remove the leftover commented-out beforeEach/afterEach block from the
  old session-per-test model
- Drop the now-obsolete MODULARIZATION row from CLAUDE.md
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.

5 participants