Skip to content

Allow tests for Extracted & Builtin LTI XBlock #36020

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.

Sign up for GitHub

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

Jump to bottom
Open
ttqureshi wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Allow tests for Extracted & Builtin LTI XBlock #36020

ttqureshi wants to merge 5 commits into from
+135 −29

Conversation

@ttqureshi
Copy link
Member

@ttqureshi ttqureshi commented Dec 12, 2024 •
edited
Loading

Test the LTI XBlock in both built-in and extracted modes to keep them in sync.

Related to: #36281

@ttqureshi ttqureshi marked this pull request as draft December 12, 2024 08:31
@ttqureshi ttqureshi marked this pull request as ready for review December 12, 2024 08:37
@ttqureshi ttqureshi force-pushed the ttqureshi/test-lti-block branch from ba481cd to 8343717 Compare December 12, 2024 08:40
@ttqureshi ttqureshi marked this pull request as draft December 12, 2024 09:21
@ttqureshi ttqureshi force-pushed the ttqureshi/test-lti-block branch 4 times, most recently from d132ef3 to eaf2743 Compare February 17, 2025 10:19
@ttqureshi ttqureshi force-pushed the ttqureshi/test-lti-block branch 4 times, most recently from 8a886fd to 48be2e4 Compare February 21, 2025 05:57
@ttqureshi ttqureshi force-pushed the ttqureshi/test-lti-block branch from cf6d9cc to 995261a Compare March 17, 2025 09:35
@ttqureshi
Copy link
Member Author

ttqureshi commented Apr 3, 2025 •
edited
Loading

@kdmccormick
test_number_mongo_calls this test case is failing with the test params expecting 37 mongo calls as in 2nd, 3rd and 4th test case parameters.

With the flag (USE_EXTRACTED_LTI_BLOCK) set to True, 36 calls are made and thus assertion fails. I have tried to figure out the reason but couldn't get any clue.

I'm sharing the output diff here. Please take a look, and guide me what could be the possible way-outs.

cc: @feanil @farhan

@ttqureshi ttqureshi force-pushed the ttqureshi/test-lti-block branch from f5560d2 to efc6cb7 Compare April 4, 2025 13:25
@farhan
Copy link
Contributor

farhan commented Apr 5, 2025

@ttqureshi

test_number_mongo_calls this test case is failing with the test params expecting 37 mongo calls as in 2nd, 3rd and 4th test case parameters.

  1. We are not adding the EmptyDataRawMixin in the extracted XBlock.
  2. EmptyDataRawMixin has data String field which is missing in the extracted XBlock
  3. This 1 less field is causing 1 less mongo DB call. So either
    a. add this data field for initial extraction to make exact code
    b. or decrease 1 count in test case

Note: I don't have technical depth of the mongo calls working, it needs to be explored

@ttqureshi
Copy link
Member Author

ttqureshi commented Apr 7, 2025 •
edited
Loading

@farhan
Thanks for giving the pointers.

  1. We are not adding the EmptyDataRawMixin in the extracted XBlock.
  2. EmptyDataRawMixin has data String field which is missing in the extracted XBlock
  3. This 1 less field is causing 1 less mongo DB call. So either
    a. add this data field for initial extraction to make exact code
    b. or decrease 1 count in test case

Seems like this is causing the issue. Let me try things around first, and IMO decreasing the count by 1 in the test case (inside if condition) sounds more reasonable than adding extra data field that is not required anywhere in the extracted LTIBlock.

@ttqureshi ttqureshi mentioned this pull request Apr 8, 2025
Merged
8 tasks
@ttqureshi ttqureshi force-pushed the ttqureshi/test-lti-block branch 3 times, most recently from 373aa3e to 58f8e54 Compare April 10, 2025 13:38
@kdmccormick kdmccormick self-requested a review April 10, 2025 17:14
@ttqureshi ttqureshi force-pushed the ttqureshi/test-lti-block branch 2 times, most recently from 324c681 to 9c4c049 Compare April 15, 2025 09:46
@ttqureshi ttqureshi mentioned this pull request Apr 15, 2025
Closed
2 tasks
@ttqureshi ttqureshi force-pushed the ttqureshi/test-lti-block branch from 9c4c049 to b5f2551 Compare April 16, 2025 13:44
@ttqureshi ttqureshi mentioned this pull request May 19, 2025
Closed
@kdmccormick kdmccormick self-assigned this Aug 1, 2025
@kdmccormick kdmccormick removed their request for review August 14, 2025 17:03
@ttqureshi ttqureshi force-pushed the ttqureshi/test-lti-block branch from 4164531 to b2ffa96 Compare September 16, 2025 08:36
@ttqureshi ttqureshi added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Jan 5, 2026
@open-craft-grove
Copy link

open-craft-grove commented Jan 5, 2026

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@open-craft-grove
Copy link

open-craft-grove commented Jan 5, 2026

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@ttqureshi ttqureshi force-pushed the ttqureshi/test-lti-block branch 2 times, most recently from f9e7c57 to f6edf0b Compare January 6, 2026 07:21
@open-craft-grove
Copy link

open-craft-grove commented Jan 6, 2026

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@open-craft-grove
Copy link

open-craft-grove commented Jan 6, 2026

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@ttqureshi ttqureshi mentioned this pull request Jan 6, 2026
Closed
@open-craft-grove
Copy link

open-craft-grove commented Jan 6, 2026

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@open-craft-grove
Copy link

open-craft-grove commented Jan 6, 2026

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@ttqureshi
Copy link
Member Author

ttqureshi commented Jan 6, 2026

@farhan

We per discussion this XBlock needs to be properly tested on the Content Library - V2 like we have tested the other XBlocks.

Here is some crash logs while testing on content-library:

I've opened an issue to fix this: openedx/xblocks-contrib#132

@ttqureshi ttqureshi force-pushed the ttqureshi/test-lti-block branch from f6edf0b to de1ae30 Compare January 14, 2026 13:47
@open-craft-grove
Copy link

open-craft-grove commented Jan 14, 2026

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@ttqureshi ttqureshi force-pushed the ttqureshi/test-lti-block branch from de1ae30 to c215ec0 Compare January 15, 2026 13:11
@open-craft-grove
Copy link

open-craft-grove commented Jan 15, 2026

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@open-craft-grove
Copy link

open-craft-grove commented Jan 15, 2026

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@ttqureshi ttqureshi closed this Jan 15, 2026
@github-project-automation github-project-automation bot moved this from Blocked to Done in Contributions Jan 15, 2026
@ttqureshi ttqureshi reopened this Jan 15, 2026
@open-craft-grove
Copy link

open-craft-grove commented Jan 15, 2026

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@ttqureshi ttqureshi requested a review from farhan January 16, 2026 09:47
@ttqureshi ttqureshi removed the blocked by other work PR cannot be finished until other work is complete label Jan 16, 2026
salman2013
salman2013 approved these changes Jan 19, 2026
View reviewed changes
Copy link
Contributor

@salman2013 salman2013 left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@salman2013 salman2013 salman2013 approved these changes

@farhan farhan farhan approved these changes

@irtazaakram irtazaakram Awaiting requested review from irtazaakram irtazaakram is a code owner

Assignees

@ttqureshi ttqureshi

Labels

create-sandbox open-craft-grove should create a sandbox environment from this PR open-source-contribution PR author is not from Axim or 2U

Projects

Contributions
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow tests for Extracted & BuiltIn LTI XBlock

7 participants

@ttqureshi @farhan @openedx-webhooks @mphilbrick211 @open-craft-grove @salman2013 @kdmccormick