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

Add Unit Testing Guidelines to CONTRIBUTING.md for Scribe Android #228

Merged
merged 4 commits into from
Oct 30, 2024

Conversation

KesharwaniArpita
Copy link
Contributor

@KesharwaniArpita KesharwaniArpita commented Oct 27, 2024

Contributor checklist


Description

This PR enhances the CONTRIBUTING.md file by adding comprehensive unit testing guidelines for the Scribe Android project. The added section provides structured guidance on writing, organizing, and naming unit tests, with best practices that will improve test consistency, readability, and effectiveness across the codebase.

Key Changes:

  1. Importance of Unit Testing:

    • A brief overview explaining why unit tests are essential for reliability and maintainability.
  2. Project Structure:

    • Guidelines on where to place test files and new classes for testing, following a consistent folder structure.
  3. Naming Conventions:

    • Descriptive naming rules for test classes and methods to clearly indicate purpose and expected outcomes.
  4. Scope and Focus:

    • Emphasis on single-responsibility tests, with setup/teardown instructions for isolated testing.
    • Recommendations on using mocks for dependencies.
  5. Writing Effective Tests:

    • Using the Arrange-Act-Assert (AAA) pattern to organize test logic clearly.
    • Tips for edge-case coverage and meaningful assertions.
  6. Documentation for Tests:

    • Guidance on adding comments and failure messages for complex test logic.

Motivation:

These conventions will help maintain a high standard of testing in the Scribe Android codebase, providing clarity for both current and new contributors and reducing maintenance overhead over time.

CC: @albendz Please review the added conventions and suggest any adjustments if needed.

Related issue

Copy link

github-actions bot commented Oct 27, 2024

Thank you for the pull request!

The Scribe team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Android rooms once you're in. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you!

Maintainer checklist

  • The linting and formatting workflows within the PR checks do not indicate new errors in the files changed

  • The CHANGELOG has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

Copy link
Contributor

@albendz albendz left a comment

Choose a reason for hiding this comment

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

This covers what I was thinking of when I created the issue - thank you for creating and adding this content!

CONTRIBUTING.md Outdated
}
```

- **Mocking**: Use mocks (e.g., Mockito) to isolate the unit under test, especially with dependencies like databases, network requests, or services.
Copy link
Contributor

Choose a reason for hiding this comment

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

This project is setup with MockK so I recommend referring to that instead of Mockito

CONTRIBUTING.md Outdated
- **Comments**: Add comments when test logic is complex or non-intuitive.
- **Assertions**: Use descriptive assertion methods (`assertTrue`, `assertEquals`, etc.) for clarity and include failure messages for custom assertions if necessary.

Following these conventions ensures consistent, readable, and reliable unit tests, enhancing the quality of the Scribe Android codebase.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's good to highlight the importance of testing but this is very similar to the opening content of this section and could be removed. The shorter the docs are, the more likely people are to finish reading through them.

CONTRIBUTING.md Outdated

#### 1. Project Structure for Unit Tests

- **Location**: Place all unit tests in the `src/test/java` directory, mirroring the structure of the `src/main/java` directory. For new classes or features, ensure their corresponding test classes follow the same package structure.
Copy link
Contributor

Choose a reason for hiding this comment

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

This must be an Android project structure thing - I'm newer to Android - why is kotlin code put under java directories?

The unit tests I added as an example are under src/test/kotlin so a follow up will need to be moving the example to the correct structure if src/test/java is the desired structure.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not actually sure on this either, @albendz :) Was the structure of the project that was forked. Maybe we could look into this in #223 where some renaming will happen 🤔

- **Class Files**: Each class in `src/main/java` should have a dedicated test file in `src/test/java`, named by appending `Test` to the class name (e.g., `UserManager` → `UserManagerTest`).
- **New Classes for Testing**: When a new utility or helper class is needed specifically for testing, place it under `src/test/java/utils` or `src/test/java/helpers`.

#### 2. Naming Conventions for Tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for adding examples in this section

@andrewtavis andrewtavis added the hacktoberfest-accepted Accepted as a part of Hacktoberfest label Oct 30, 2024
Copy link
Member

@andrewtavis andrewtavis left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, @KesharwaniArpita :) This will definitely come in handy as we expand out the testing of the project.

@andrewtavis andrewtavis merged commit 8137617 into scribe-org:main Oct 30, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accepted as a part of Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants