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.

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

ux: Better assignment tests UX #384

Merged
merged 1 commit into from
Mar 29, 2024
Merged

ux: Better assignment tests UX #384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 4 additions & 3 deletions lms/lmstests/public/unittests/services.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,10 @@ def _handle_failed_to_execute_tests(self, raw_results: bytes) -> None:
def _handle_result(
self, case_name: str, result: junitparser.Element,
) -> None:
message = ' '.join(
elem[1].replace('\n', '')
for elem in result._elem.items()
# Extract only the Assertion Message
message = '\n'.join(
msg_.partition("\n")[0].removeprefix("AssertionError: ")
for _type, msg_ in result._elem.items()
)
self._logger.info(
'Create comment on test %s solution %s.',
Expand Down
34 changes: 17 additions & 17 deletions lms/lmsweb/translations/he/LC_MESSAGES/messages.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ msgid ""
msgstr ""
"Project-Id-Version: 1.0\n"
"Report-Msgid-Bugs-To: EMAIL@ADDRESS\n"
"POT-Creation-Date: 2024-03-29 05:54+0300\n"
"POT-Creation-Date: 2024-03-29 21:14+0300\n"
"PO-Revision-Date: 2024-03-23 18:59+0300\n"
"Last-Translator: Yam Mesicka\n"
"Language: he\n"
Expand Down Expand Up @@ -48,31 +48,31 @@ msgstr "הבודק האוטומטי נכשל ב־%(number)d דוגמאות בת
msgid "Woah! Did you check your code?"
msgstr "ווהו! בדקת את הקוד שלך?"

#: lms/lmsweb/views.py:183
#: lms/lmsweb/views.py:181
msgid "Can not register now"
msgstr "לא ניתן להירשם כעת"

#: lms/lmsweb/views.py:205
#: lms/lmsweb/views.py:203
msgid "Registration successfully"
msgstr "ההרשמה בוצעה בהצלחה"

#: lms/lmsweb/views.py:233
#: lms/lmsweb/views.py:231
msgid "The confirmation link is expired, new link has been sent to your email"
msgstr "קישור האימות פג תוקף, קישור חדש נשלח אל תיבת המייל שלך"

#: lms/lmsweb/views.py:252
#: lms/lmsweb/views.py:250
msgid "Your user has been successfully confirmed, you can now login"
msgstr "המשתמש שלך אומת בהצלחה, כעת אתה יכול להתחבר למערכת"

#: lms/lmsweb/views.py:277 lms/lmsweb/views.py:345
#: lms/lmsweb/views.py:275 lms/lmsweb/views.py:343
msgid "Your password has successfully changed"
msgstr "הסיסמה שלך שונתה בהצלחה"

#: lms/lmsweb/views.py:295
#: lms/lmsweb/views.py:293
msgid "Password reset link has successfully sent"
msgstr "קישור לאיפוס הסיסמה נשלח בהצלחה"

#: lms/lmsweb/views.py:321
#: lms/lmsweb/views.py:319
msgid "Reset password link is expired"
msgstr "הקישור לאיפוס הסיסמה פג תוקף"

Expand Down Expand Up @@ -413,7 +413,7 @@ msgid "Checker"
msgstr "בודק"

#: lms/templates/user.html:37 lms/templates/view.html:25
#: lms/templates/view.html:114
#: lms/templates/view.html:118
msgid "Assessment"
msgstr "הערכה מילולית"

Expand Down Expand Up @@ -485,23 +485,23 @@ msgstr "סיסמה נוכחית"
msgid "Finish Checking"
msgstr "סיום בדיקה"

#: lms/templates/view.html:86
#: lms/templates/view.html:88
msgid "Automatic Checking"
msgstr "בדיקות אוטומטיות"

#: lms/templates/view.html:93
msgid "Error"
msgstr "כישלון"
msgid "Test failed"
msgstr "בדיקה נכשלה"

#: lms/templates/view.html:98
msgid "Staff Error"
msgstr "כישלון סגל"
#: lms/templates/view.html:101
msgid "Staff traceback"
msgstr "Traceback לצוות"

#: lms/templates/view.html:130
#: lms/templates/view.html:134
msgid "Comments for this exercise"
msgstr "הערות על התרגיל"

#: lms/templates/view.html:138
#: lms/templates/view.html:142
msgid "Done Checking"
msgstr "סיים לבדוק"

Expand Down
2 changes: 0 additions & 2 deletions lms/lmsweb/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

import arrow # type: ignore
from flask import (
# Response,
jsonify,
make_response,
render_template,
Expand Down Expand Up @@ -40,7 +39,6 @@
)
from lms.lmsweb import (
babel,
# http_basic_auth,
limiter,
routes,
webapp,
Expand Down
111 changes: 92 additions & 19 deletions lms/static/my.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,17 +467,24 @@ button.our-button-narrow, a.our-button-narrow {
border-bottom-left-radius: 0.25rem;
}

.code-view-container {
.code-review-container {
position: relative;
direction: ltr;
text-align: left;
height: min-content;
display: flex;
overflow-x: hidden;
flex-flow: column wrap;
flex-direction: column;
flex-basis: 70vw;
flex-grow: 4;
}
Comment on lines +470 to +477
Copy link

Choose a reason for hiding this comment

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

suggestion (code_clarification): Consider using a more descriptive class name.

The class name '.code-review-container' could be more specific to its purpose or content, enhancing readability and maintainability.


.code-view-container {
border: 1px solid #e1e4e8;
direction: ltr;
display: flex;
flex-flow: column wrap;
margin-block-end: 2rem;
overflow-x: hidden;
text-align: left;
width: 100%;
}

#code-view {
Expand Down Expand Up @@ -612,36 +619,98 @@ pre {
}

.test-results {
direction: rtl;
text-align: right;
font-family: Arial, sans-serif;
display: flex;
width: 100%;
flex-direction: column;
margin: auto;
box-shadow: 0 4px 6px rgba(0, 0, 0, 0.1);
}

.test-result-title {
.test-results-header {
background-color: #F9FAFB;
text-align: center;
text-decoration: underline;
margin-top: 20px;
color: #333;
padding: 0.5rem 0;
border-radius: 6px 6px 0 0;
font-size: 1.1rem;
}

.test-results-list {
list-style: none;
padding: 0;
margin: 0;
}

.test-result {
padding: 10px;
margin: 5px 0;
background-color: #FFFFFF;
border-radius: 6px;
border: 1px solid #E2E8F0;
display: flex;
flex-direction: column;
border: 1px solid #c2bdbd;
padding: 1.5rem;
transition: box-shadow 0.2s ease;
width: 100%;
}

.test-result .test-name {
border-bottom: 1px solid #c2bdbd;
margin-bottom: 12px;
padding-bottom: 6px;
.test-header {
display: flex;
justify-content: space-between;
align-items: center;
}

.test-name {
font-weight: 600;
color: #1A202C;
}

.test-status.failure {
background-color: #F56565;
border-radius: 12px;
color: #FFFFFF;
font-size: 0.85rem;
padding: 0.25rem 0.5rem;
}

.test-result .title {
.error-description {
background-color: #F7FAFC;
border-radius: 4px;
color: #2D3748;
display: flex;
line-height: 1.5;
margin: 0.5rem 0;
padding: 0.75rem;
}

.staff-failure .title {
font-weight: bold;
display: flex;
margin-bottom: 5px;
}

.stack-trace pre {
background-color: #EDF2F7;
color: #2D3748;
padding: 0.75rem;
border-radius: 4px;
overflow-x: auto;
}

.stack-trace code {
width: 100%;
background-color: #F8F9FA;
color: #333;
padding: 10px;
border-radius: 4px;
display: flex;
flex-direction: column;
white-space: pre;
}

@media (max-width: 768px) {
.test-result {
padding: 1rem; /* less padding on smaller screens */
}
}

.stack-trace {
Expand Down Expand Up @@ -809,7 +878,7 @@ code > code:last-of-type{
*/
pre[class*="language-"] {
/* Override prism.js */
padding: 0 !important;
padding: 0.5rem !important;
Copy link

Choose a reason for hiding this comment

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

suggestion (code_refinement): Use of !important in CSS should be minimized.

Consider alternative methods to increase specificity without using !important, as it can make future CSS maintenance challenging.

margin: 0 !important;
}

Expand All @@ -829,6 +898,10 @@ code#user-code {
justify-content: start;
cursor: pointer;
width: 100%;
/* Nice trick: with flex-grow: 1; and width: 0;
* the line fits the container width
* */
width: 0;
flex-grow: 1;
}

Expand Down
84 changes: 44 additions & 40 deletions lms/templates/view.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,58 +50,62 @@ <h1>{{ _('Exercise view') }} {{ solution['exercise']['number'] }}: {{ solution['
{%- endif %}
</div>
<div id="view-body">
<div class="code-view-container">
<div id="code-toolbar">
<div id="number-of-lines">{{ current_file.code.splitlines() | length }} lines</div>
<div id="code-toolbar-buttons">
{% if config.SHAREABLE_SOLUTIONS and not shared_url %}
<div id="share-solution" class="code-toolbar-button glowing">
<div id="shared-text">
<button id="share-action" title="Share solution"><i class="fa fa-share-alt"></i></button>
<div class="code-review-container">
<div class="code-view-container">
<div id="code-toolbar">
<div id="number-of-lines">{{ current_file.code.splitlines() | length }} lines</div>
<div id="code-toolbar-buttons">
{% if config.SHAREABLE_SOLUTIONS and not shared_url %}
<div id="share-solution" class="code-toolbar-button glowing">
<div id="shared-text">
<button id="share-action" title="Share solution"><i class="fa fa-share-alt"></i></button>
</div>
<div id="shared-box" class="glowing d-none">
<button id="cancel-share" class="share-button" title="Disable share solution"><i class="fa fa-trash"></i></button>
<button id="copy-link" class="share-button" title="Copy shared link"><i class="fa fa-copy"></i></button>
<input id="shareable-link" class="form-control" type="text" readonly>
</div>
</div>
<div id="shared-box" class="glowing d-none">
<button id="cancel-share" class="share-button" title="Disable share solution"><i class="fa fa-trash"></i></button>
<button id="copy-link" class="share-button" title="Copy shared link"><i class="fa fa-copy"></i></button>
<input id="shareable-link" class="form-control" type="text" readonly>
{% endif -%}
<div id="download-code" class="code-toolbar-button glowing">
<form>
<button id="download-solution" formaction="/download/{% if shared_url %}{{ shared_url }}{% else %}{{ solution['id'] }}{% endif %}">
<i class="fa fa-download"></i>
</button>
</form>
</div>
<div id="copy-code" class="code-toolbar-button glowing">
<button id="copy-button" class="button-copy" title="Copy code"><i class="fa fa-copy"></i></button>
</div>
</div>
{% endif -%}
<div id="download-code" class="code-toolbar-button glowing">
<form>
<button id="download-solution" formaction="/download/{% if shared_url %}{{ shared_url }}{% else %}{{ solution['id'] }}{% endif %}">
<i class="fa fa-download"></i>
</button>
</form>
</div>
<div id="copy-code" class="code-toolbar-button glowing">
<button id="copy-button" class="button-copy" title="Copy code"><i class="fa fa-copy"></i></button>
</div>
</div>
</div>
<div id="code-view" data-id="{{ solution['id'] }}" data-file="{{ current_file.id }}" data-exercise="{{ solution['exercise']['id'] }}" data-role="{{ role }}" data-solver="{{ solution['solver']['fullname'] }}" data-solver-id="{{ solution['solver']['id'] }}" data-allowed-comment="{{ config['USERS_COMMENTS'] | lower }}">
<pre><code id="user-code" class="language-{{ current_file | language_name }} highlight">{% if current_file | language_name in image_extensions %}<div class="code-image-block"><img class="code-image" src="data:{{ current_file | mime_type }};base64,{{ current_file.code }}"></div>{% else %}{{- current_file.code | trim(chars=' ') | e -}}{% endif %}</code></pre>
<div id="code-view" data-id="{{ solution['id'] }}" data-file="{{ current_file.id }}" data-exercise="{{ solution['exercise']['id'] }}" data-role="{{ role }}" data-solver="{{ solution['solver']['fullname'] }}" data-solver-id="{{ solution['solver']['id'] }}" data-allowed-comment="{{ config['USERS_COMMENTS'] | lower }}">
<pre><code id="user-code" class="language-{{ current_file | language_name }} highlight">{% if current_file | language_name in image_extensions %}<div class="code-image-block"><img class="code-image" src="data:{{ current_file | mime_type }};base64,{{ current_file.code }}"></div>{% else %}{{- current_file.code | trim(chars=' ') | e -}}{% endif %}</code></pre>
</div>
</div>
{% if test_results and not shared_url %}
<div class="test-results">
<h3 class="test-result-title">{{ _('Automatic Checking') }}</h3>
<div class="test-results {{ direction }}">
<div class="test-results-header">{{ _('Automatic Checking') }}</div>
<ol class="test-results-list">
{%- for test_result in test_results %}
<li class="test-result">
<h5 class="test-name">
{{ test_result.pretty_test_name | e }}
</h5>
<span class="title">{{ _('Error') }}:</span>
<span>
<pre><code class="language-{{ current_file | language_name }} highlight">{{ test_result.user_message | e }}</code></pre>
</span>
<div class="test-header">
<span class="test-status failure">{{ _('Test failed') }}: </span>
<span class="test-name">{{ test_result.pretty_test_name | e }}</span>
</div>
<div class="test-description">
<span class="error-description">{{ test_result.user_message | e }}</span>
</div>
{% if is_manager %}
<span class="title">{{ _('Staff Error') }}:</span>
<span class="stack-trace">
<pre><code class="language-{{ current_file | language_name }} highlight">{{ test_result.staff_message | e }}</code></pre>
</span>
<div class="staff-failure">
<span class="title">{{ _('Staff traceback') }}:</span>
<span class="stack-trace">
<pre><code class="language-{{ current_file | language_name }} highlight">{{ test_result.staff_message | e }}</code></pre>
</span>
</div>
{% endif %}
</li>
{% endfor -%}
{% endfor %}
</ol>
</div>
{% endif %}
Expand Down
3 changes: 1 addition & 2 deletions tests/test_exercise_unit_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@ def _verify_comments():
first = auto_comments[0]
assert first.exercise_test_name.test_name == 'test_check_bar_bar'
assert first.exercise_test_name.pretty_test_name == 'שם כזה מגניב 2'
expected = ("AssertionError: איזה ברברון"
"assert 'bar' == 'barbaron' - barbaron + bar")
expected = "איזה ברברון"
Copy link

Choose a reason for hiding this comment

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

suggestion (testing): Consider adding a test for multi-line assertion messages.

The change in services.py now handles assertion messages differently, focusing on the first line only. It would be beneficial to ensure that this behavior is explicitly tested, especially for multi-line assertion messages, to verify that only the intended portion of the message is extracted.

Copy link

Choose a reason for hiding this comment

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

suggestion (testing): Ensure the test covers various assertion message formats.

Given the modification in services.py to extract and format assertion messages, it's crucial to test with various assertion message formats to confirm the robustness of the new logic. This includes testing with messages that do not start with 'AssertionError: ' to ensure they are handled correctly.

assert expected == first.user_message
assert "foo('bar') == 'barbaron'" in first.staff_message

Expand Down
Loading