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

Test PR | Enable Extracted Word Cloud #35983

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ jobs:
name: ${{ matrix.shard_name }}(py=${{ matrix.python-version }},dj=${{ matrix.django-version }},mongo=${{ matrix.mongo-version }})
runs-on: ${{ matrix.os-version }}
strategy:
fail-fast: false
matrix:
python-version:
- "3.11"
Expand Down
107 changes: 80 additions & 27 deletions lms/djangoapps/courseware/tests/test_word_cloud.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
"""Word cloud integration tests using mongo modulestore."""


import pytest

import json
import re
from uuid import UUID
from operator import itemgetter
from unittest.mock import patch

import pytest
from django.conf import settings

from common.djangoapps.student.tests.factories import RequestFactoryNoCsrf
from lms.djangoapps.courseware import block_render as render
from openedx.core.lib.url_utils import quote_slashes
# noinspection PyUnresolvedReferences
from xmodule.tests.helpers import override_descriptor_system # pylint: disable=unused-import
from xmodule.tests.helpers import override_descriptor_system, mock_render_template # pylint: disable=unused-import
from xmodule.x_module import STUDENT_VIEW

from .helpers import BaseTestXmodule


Expand All @@ -18,6 +22,10 @@ class TestWordCloud(BaseTestXmodule):
"""Integration test for Word Cloud Block."""
CATEGORY = "word_cloud"

def setUp(self):
super().setUp()
self.request_factory = RequestFactoryNoCsrf()

def _get_users_state(self):
"""Return current state for each user:

Expand All @@ -27,7 +35,23 @@ def _get_users_state(self):
users_state = {}

for user in self.users:
response = self.clients[user.username].post(self.get_url('get_state'))
if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK:
request = self.request_factory.post(
'/',
content_type='application/json',
HTTP_X_REQUESTED_WITH='XMLHttpRequest'
)
request.user = user
request.session = {}
response = render.handle_xblock_callback(
request,
str(self.course.id),
quote_slashes(self.item_url),
'handle_get_state',
'',
)
else:
response = self.clients[user.username].post(self.get_url('get_state'))
users_state[user.username] = json.loads(response.content.decode('utf-8'))

return users_state
Expand All @@ -40,11 +64,28 @@ def _post_words(self, words):
users_state = {}

for user in self.users:
response = self.clients[user.username].post(
self.get_url('submit'),
{'student_words[]': words},
HTTP_X_REQUESTED_WITH='XMLHttpRequest'
)
if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK:
request = self.request_factory.post(
'/',
data={'student_words': words},
content_type='application/json',
HTTP_X_REQUESTED_WITH='XMLHttpRequest'
)
request.user = user
request.session = {}
response = render.handle_xblock_callback(
request,
str(self.course.id),
quote_slashes(self.item_url),
'handle_submit_state',
'',
)
else:
response = self.clients[user.username].post(
self.get_url('submit'),
{'student_words[]': words},
HTTP_X_REQUESTED_WITH='XMLHttpRequest'
)
users_state[user.username] = json.loads(response.content.decode('utf-8'))

return users_state
Expand Down Expand Up @@ -202,31 +243,43 @@ def test_handle_ajax_incorrect_dispatch(self):
for user in self.users
}

status_codes = {response.status_code for response in responses.values()}
assert status_codes.pop() == 200
if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK:
for username, response in responses.items():
self.assertEqual(response.status_code, 404)
else:
status_codes = {response.status_code for response in responses.values()}
assert status_codes.pop() == 200

for user in self.users:
self.assertDictEqual(
json.loads(responses[user.username].content.decode('utf-8')),
{
'status': 'fail',
'error': 'Unknown Command!'
}
)
for user in self.users:
self.assertDictEqual(
json.loads(responses[user.username].content.decode('utf-8')),
{
'status': 'fail',
'error': 'Unknown Command!'
}
)

def test_word_cloud_constructor(self):
@patch('xblock.utils.resources.ResourceLoader.render_django_template', side_effect=mock_render_template)
def test_word_cloud_constructor(self, mock_render_django_template):
"""
Make sure that all parameters extracted correctly from xml.
"""
fragment = self.runtime.render(self.block, STUDENT_VIEW)
expected_context = {
'ajax_url': self.block.ajax_url,
'display_name': self.block.display_name,
'instructions': self.block.instructions,
'element_class': self.block.location.block_type,
'element_id': self.block.location.html_id(),
'element_class': self.block.scope_ids.block_type,
'num_inputs': 5, # default value
'submitted': False, # default value,
}

assert fragment.content == self.runtime.render_template('word_cloud.html', expected_context)
if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK:
expected_context['range_num_inputs'] = range(5)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not check the block type in this case?

Copy link
Contributor Author

@farhan farhan Jan 22, 2025

Choose a reason for hiding this comment

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

Fixed, checking for both (Builtin & Extracted) XBlocks now.

uuid_str = re.search(r"UUID\('([a-f0-9\-]+)'\)", fragment.content).group(1)
expected_context['element_id'] = UUID(uuid_str)
mock_render_django_template.assert_called_once()
assert fragment.content == self.runtime.render_template('templates/word_cloud.html', expected_context)
else:
expected_context['ajax_url'] = self.block.ajax_url
expected_context['element_id'] = self.block.location.html_id()
assert fragment.content == self.runtime.render_template('word_cloud.html', expected_context)
2 changes: 1 addition & 1 deletion lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -5573,7 +5573,7 @@ def _should_send_learning_badge_events(settings):
# .. toggle_warning: Not production-ready until https://github.com/openedx/edx-platform/issues/34840 is done.
# .. toggle_creation_date: 2024-11-10
# .. toggle_target_removal_date: 2025-06-01
USE_EXTRACTED_WORD_CLOUD_BLOCK = False
USE_EXTRACTED_WORD_CLOUD_BLOCK = True

# .. toggle_name: USE_EXTRACTED_ANNOTATABLE_BLOCK
# .. toggle_default: False
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1278,7 +1278,7 @@ xblock-utils==4.0.0
# via
# edx-sga
# xblock-poll
xblocks-contrib==0.1.0
git+https://github.com/openedx/xblocks-contrib.git@farhan/word-cloud-xblock
# via -r requirements/edx/bundled.in
xmlsec==1.3.14
# via python3-saml
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/development.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2282,7 +2282,7 @@ xblock-utils==4.0.0
# -r requirements/edx/testing.txt
# edx-sga
# xblock-poll
xblocks-contrib==0.1.0
git+https://github.com/openedx/xblocks-contrib.git@farhan/word-cloud-xblock
# via
# -r requirements/edx/doc.txt
# -r requirements/edx/testing.txt
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/doc.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1606,7 +1606,7 @@ xblock-utils==4.0.0
# -r requirements/edx/base.txt
# edx-sga
# xblock-poll
xblocks-contrib==0.1.0
git+https://github.com/openedx/xblocks-contrib.git@farhan/word-cloud-xblock
# via -r requirements/edx/base.txt
xmlsec==1.3.14
# via
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/testing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1696,7 +1696,7 @@ xblock-utils==4.0.0
# -r requirements/edx/base.txt
# edx-sga
# xblock-poll
xblocks-contrib==0.1.0
git+https://github.com/openedx/xblocks-contrib.git@farhan/word-cloud-xblock
# via -r requirements/edx/base.txt
xmlsec==1.3.14
# via
Expand Down
39 changes: 33 additions & 6 deletions xmodule/tests/test_word_cloud.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
"""Test for Word Cloud Block functional logic."""

import json
import os
from unittest.mock import Mock

from django.conf import settings
from django.test import TestCase
from fs.memoryfs import MemoryFS
from lxml import etree
from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
from webob.multidict import MultiDict
from xblock.field_data import DictFieldData
from xblock.fields import ScopeIds

from xmodule.word_cloud_block import WordCloudBlock
from . import get_test_descriptor_system, get_test_system
Expand Down Expand Up @@ -42,7 +45,11 @@ def test_xml_import_export_cycle(self):

olx_element = etree.fromstring(original_xml)
runtime.id_generator = Mock()
block = WordCloudBlock.parse_xml(olx_element, runtime, None)

def_id = runtime.id_generator.create_definition(olx_element.tag, olx_element.get('url_name'))
keys = ScopeIds(None, olx_element.tag, def_id, runtime.id_generator.create_usage(def_id))
block = WordCloudBlock.parse_xml(olx_element, runtime, keys)
Copy link
Contributor Author

@farhan farhan Jan 24, 2025

Choose a reason for hiding this comment

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

As per discussion in pair programming session (@kdmccormick , @ttqureshi ) we are creating scope ids here which shouldn't happen and is a potential bug


block.location = BlockUsageLocator(
CourseLocator('org', 'course', 'run', branch='revision'), 'word_cloud', 'block_id'
)
Expand All @@ -53,18 +60,34 @@ def test_xml_import_export_cycle(self):
assert block.num_inputs == 3
assert block.num_top_words == 100

node = etree.Element("unknown_root")
# This will export the olx to a separate file.
block.add_xml_to_node(node)
if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK:
filepath = 'word_cloud/block_id.xml'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use the same method as below to export the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because add_xml_to_node is defined in XmlMixin class.
BuiltIn XBlocks are using this mixin

In Extracted XBlocks we are preferring to use Xblock.core functionalities and using export_to_xml method which doesn't have export_to_file funcationlity like add_xml_to_node method of XmlMixin has

runtime.export_fs.makedirs(os.path.dirname(filepath), recreate=True)
with runtime.export_fs.open(filepath, 'wb') as fileObj:
runtime.export_to_xml(block, fileObj)
else:
node = etree.Element("unknown_root")
# This will export the olx to a separate file.
block.add_xml_to_node(node)

with runtime.export_fs.open('word_cloud/block_id.xml') as f:
exported_xml = f.read()

if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK:
exported_xml_tree = etree.fromstring(exported_xml.encode('utf-8'))
etree.cleanup_namespaces(exported_xml_tree)
if 'xblock-family' in exported_xml_tree.attrib:
del exported_xml_tree.attrib['xblock-family']
exported_xml = etree.tostring(exported_xml_tree, encoding='unicode', pretty_print=True)

assert exported_xml == original_xml

def test_bad_ajax_request(self):
"""
Make sure that answer for incorrect request is error json.
"""
if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK:
return

module_system = get_test_system()
block = WordCloudBlock(module_system, DictFieldData(self.raw_field_data), Mock())
Expand All @@ -83,8 +106,12 @@ def test_good_ajax_request(self):
module_system = get_test_system()
block = WordCloudBlock(module_system, DictFieldData(self.raw_field_data), Mock())

post_data = MultiDict(('student_words[]', word) for word in ['cat', 'cat', 'dog', 'sun'])
response = json.loads(block.handle_ajax('submit', post_data))
if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK:
post_data = {'student_words': ['cat', 'cat', 'dog', 'sun']}
response = block.submit_state(post_data)
else:
post_data = MultiDict(('student_words[]', word) for word in ['cat', 'cat', 'dog', 'sun'])
response = json.loads(block.handle_ajax('submit', post_data))
assert response['status'] == 'success'
assert response['submitted'] is True
assert response['total_count'] == 22
Expand Down
Loading