Skip to content

Conversation

@ryan331913
Copy link
Contributor

This PR add the flashcards / stats related tests in #56

@0010aor
Copy link
Owner

0010aor commented Apr 29, 2025

Thank you for your contribution I'll take a close look at the changes. 🙌

Copy link
Owner

@0010aor 0010aor left a comment

Choose a reason for hiding this comment

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

Thank you so much for your contribution! 🙌
I can really see the hard work you put into this.

result = session.exec(statement).first()
if result:
return result[1], result[0]
# Not used
Copy link
Owner

Choose a reason for hiding this comment

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

nice catch, I think we can completely remove it

) -> PracticeSession:
return session.get(PracticeSession, practice_session_id)
# Not used
# def get_session_statistics(
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove this too.

assert len(content["data"]) <= 3


def test_read_cards_wtih_nonexistent_collection(
Copy link
Owner

Choose a reason for hiding this comment

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

small typo here

assert test_card["id"] in card_ids


def test_read_cards_with_pagnation(
Copy link
Owner

Choose a reason for hiding this comment

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

small typo here

):
collection_id = test_collection["id"]
card_count = 3
# Create multiple collections
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can create a fixture for a collection that contain multiples cards

normal_user_token_headers: dict[str, str],
):
data_count = 5
# Create multiple collections
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can reuse the mention fixture here too

test_collection: dict[str, Any],
):
data_count = 3
# Create multiple collections
Copy link
Owner

Choose a reason for hiding this comment

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

same here

def test_practice_session(
db: Session, test_collection: Collection, test_multiple_cards: list[Card]
) -> PracticeSession:
session = get_or_create_practice_session(
Copy link
Owner

Choose a reason for hiding this comment

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

    _ = test_multiple_cards
    session = get_or_create_practice_session(
        session=db, collection_id=test_collection.id, user_id=test_collection.user_id
    )
    return session

to avoid unused function argument from ruff.

@ryan331913
Copy link
Contributor Author

ryan331913 commented Apr 30, 2025

@0010aor Thanks for the review, already update your suggestions.

@ryan331913
Copy link
Contributor Author

Hi @0010aor,

Is it okay for me to open a new PR for adding backend testing in Github Action Workflow?

@0010aor
Copy link
Owner

0010aor commented May 1, 2025

Hi @0010aor,

Is it okay for me to open a new PR for adding backend testing in Github Action Workflow?

That would be great! Thanks

@0010aor 0010aor merged commit b2d7b53 into 0010aor:main May 1, 2025
2 checks passed
@0010aor 0010aor linked an issue May 1, 2025 that may be closed by this pull request
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.

Implement Backend Test Coverage for Flashcards and Stats Modules

2 participants