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

docs: proctored exam testing instructions #205

Merged
merged 16 commits into from
Nov 9, 2023
Merged

Conversation

ilee2u
Copy link
Member

@ilee2u ilee2u commented Oct 26, 2023

JIRA: Link to JIRA ticket

Description: Describe in a couple of sentences what this PR adds

Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.

Dependencies: dependencies on other outstanding PRs, issues, etc.

Installation instructions: List any non-trivial installation
instructions.

Testing instructions:

  1. Open page A
  2. Do thing B
  3. Expect C to happen
  4. If D happened instead - check failed.

Merge checklist:

  • All reviewers approved
  • CI build is green
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed

Post merge:

  • Delete working branch (if not needed anymore)

@ilee2u ilee2u force-pushed the ilee2u/proctored_test_docs branch from 432ac5d to 463f75b Compare October 26, 2023 15:19
@ilee2u ilee2u changed the title docs: proctored exam testing draft docs: proctored exam testing instructions Oct 26, 2023
Copy link
Contributor

@zacharis278 zacharis278 left a comment

Choose a reason for hiding this comment

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

few comments to get started.

High level, it would be nice to make this less Proctorio specific. If we do have a few Proctorio specific cases we want to cover we keep that as a separate test. The user flow should mostly be the same regardless of the tool used.

docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
@ilee2u ilee2u force-pushed the ilee2u/proctored_test_docs branch from f48b9cc to 6f69996 Compare November 1, 2023 15:10
docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
docs/test_plan/proctored_exam.rst Show resolved Hide resolved
#. You should also have a tab open with your event bus logs


Proctored Exam Flow
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these another set of cases here for submitting an attempt that is auto verified and one needs a second review.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still think we're missing these test cases ensuring a review by the proctoring provider is received and the attempt is updated. We kind of do that as a side effect of setup for testing the instructor dashboard but I think it's best to be explicit. If someone is just testing exam flow they should be validating this case.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a test to the Proctorio section about checking that a review of the exam exists. Regardless of whether or not we're using Proctorio, wouldn't the test that we received a review by a proctoring provider be checking the exams dashboard itself?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the test I'm looking for is something that validates the ACS works as intended. So a test along the lines of "if a user submits an attempt without violation the proctoring software can update the attempt to verified" / "if a user submits with violations the proctoring software can update the attempt to review required".

I think that's a distinctly different test from testing that a verified/rejected/second review attempt can be reset in the UI since that testing instructor experience but the ACS is part of the learner / exam flow.

docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
@ilee2u ilee2u force-pushed the ilee2u/proctored_test_docs branch from 3698f55 to 9bfa421 Compare November 6, 2023 20:08
Copy link
Member

@MichaelRoytman MichaelRoytman left a comment

Choose a reason for hiding this comment

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

I know I've left a lot of feedback. I read these instructions from the perspective of a team member with limited familiarity with LTI for proctoring, Proctorio, and how to set up proctoring. I think there there is a good amount of assumed knowledge in this document that not everyone will have, and my comments reflect that. I'm not marking my feedback as blocking. I know it's a pain, but I do think this document could really benefit from being a lot more explicit.

#. As your non-staff user in your current window, and enroll in the course where you have set up your proctored exam
#. In an incognito window, login to your staff account and visit your Support Tools page and search in the email for your non-staff user. You should now see the option to change the enrollment track of that user. Change the enrollment track of the user to ""verified"" (choose any reason when prompted, it doesn't matter)
#. In that same incognito, still signed in as staff, navigate to the Instructor dashboard
#. Find or create a test course within Stage with proctoring enabled and an LTI proctoring provider chosen, certificates are enabled, and where the exams IDA waffle flag is enabled
Copy link
Member

Choose a reason for hiding this comment

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

I think that the order of these prerequisite steps is a little confusing. For example, you're enrolling a non-staff learner account in "this course" before you have set up a course with proctoring. Based on your instructions, you may set up an entirely new course in which the learner is not enrolled.

I'd recommend breaking this up into "Proctoring Set Up Steps" and "User Set Up Steps" to make this clearer.

Copy link
Member

Choose a reason for hiding this comment

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

Do you have a particular environment in mind here (e.g. stage)? Stage will have proctoring set up in a way that someone just starting in their devstack will not, so I think we should either not make references to a particular environment and link out to documentation for each environment (e.g. stage/production and development), or specify that this is just for stage/production.

Copy link
Member

Choose a reason for hiding this comment

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

"certificates are enabled" is a little vague and could mean a variety of things. I think we should be more specific here. Do you mean "where there is a verified track" or something else?

Copy link
Member

Choose a reason for hiding this comment

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

I know we don't have official documentation for this yet, but it would be good to clarify what "an LTI proctoring provider chosen" means - how does one do this or verify this? Depending on which environment this documentation applies to, you could just refer to the Proctoring Settings Page and/or reference our Local Development and LTI Configuration document for the development environment.

Copy link
Member Author

Choose a reason for hiding this comment

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

  • Changed the order of the steps as you recommended (still need to push the change)
  • I have local in mind, but based on this thread it seems like it's also fair to mention that we do have a test course in stage (at least as of now)
  • I'm honestly not too sure how certificates work so if "verified track" makes more sense I can just put that.
  • Assuming edx-exams ends up in Open edX, would it make sense to just move the instructions for Local Development and LTI Configuration to the README and just make that the source of truth?

Copy link
Contributor

Choose a reason for hiding this comment

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

for the local instructions you are correct and those do need to move as part of the release to openedx. We've got that noted as a requirement here: https://2u-internal.atlassian.net/wiki/spaces/PT/pages/626753683/Beta+Release+and+Beyond.

I do think these tests need to be done in a deployed environment and not Tutor/Devstack in order to say we're confident in releasing these features.

Copy link
Contributor

Choose a reason for hiding this comment

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

@MichaelRoytman I do think there's also going to be a gap until we get instructions in readthedocs. I don't think we should duplicate how to setup a proctored exam, a user, a proctoring provider, etc here and should just reference readthedocs where needed.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, totally agree with linking out. I didn't mean to suggest we should duplicate - just that I felt like some information was missing, which can definitely be added by referencing other documentation, like "Set up a proctored exam: ."

docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
Comment on lines +6 to +7
- `Configuring a Proctoring Provider <https://edx.readthedocs.io/projects/edx-partner-course-staff/en/latest/proctored_exams/proctored_enabling.html#configuring-proctoring-provider>`_
- `Creating a Proctored Exam <https://edx.readthedocs.io/projects/edx-partner-course-staff/en/latest/proctored_exams/pt_create.html#creating-a-proctored-exam>`_
Copy link
Member

Choose a reason for hiding this comment

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

Neither of these links is used in this document.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah these are copied over from timed_exam.rst, where it seems like they aren't used either. I see what you're saying though, I can have these links referenced in the setup instructions where it makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be how they are referenced the other docs has them right next to the step to create a proctored exam. Maybe we just want to point back to this if these instructions are needed anywhere later in the doc. If you use headings you can add internal links within the doc like this https://github.com/openedx/edx-proctoring/blob/master/docs/testing/test_plan.md?plain=1#L220. That can also be good to avoid repeating yourself if one test depends on the setup of another

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. So it's like reference materials? My bad - got confused!

docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
@ilee2u ilee2u force-pushed the ilee2u/proctored_test_docs branch from 72ee12c to d0f1977 Compare November 7, 2023 21:02
@ilee2u
Copy link
Member Author

ilee2u commented Nov 7, 2023

Made some changes based off @MichaelRoytman and @zacharis278's comments, still have some work to do though.

Copy link
Member

@MichaelRoytman MichaelRoytman 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 addressing my first set of feedback.

I added some non-blocking feedback, which you can take or leave.

I agree with Zach regarding the ACS tests.

README.rst Outdated
@@ -59,6 +59,36 @@ Devstack Set Up

You can use the make targets defined in the ``Makefile`` to interact with the running ``edx-exams`` Docker containers.

LTI Configuration for Local Development
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In order to use edx-exams as the exams backend for your courses (and to get it to wor in general), you will also need to complete the following configuration steps:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
In order to use edx-exams as the exams backend for your courses (and to get it to wor in general), you will also need to complete the following configuration steps:
In order to use edx-exams as the exams backend for your courses (and to get it to work in general), you will also need to complete the following configuration steps:

README.rst Outdated
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In order to use edx-exams as the exams backend for your courses (and to get it to wor in general), you will also need to complete the following configuration steps:

#. Add EXAMS_SERVICE_URL = 'http://host.docker.internal:18740/api/v1' to your cms and lms private.py environment.
Copy link
Member

Choose a reason for hiding this comment

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

Nit.

Suggested change
#. Add EXAMS_SERVICE_URL = 'http://host.docker.internal:18740/api/v1' to your cms and lms private.py environment.
#. Add EXAMS_SERVICE_URL = 'http://host.docker.internal:18740/api/v1' to your cms and lms private.py environment settings file.

README.rst Outdated
In order to use edx-exams as the exams backend for your courses (and to get it to wor in general), you will also need to complete the following configuration steps:

#. Add EXAMS_SERVICE_URL = 'http://host.docker.internal:18740/api/v1' to your cms and lms private.py environment.
#. In your terminal, navigate to your
Copy link
Member

Choose a reason for hiding this comment

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

It looks like you forgot to complete the sentence or finished it below and forgot to remove this fragment.

README.rst Outdated

#. Add EXAMS_SERVICE_URL = 'http://host.docker.internal:18740/api/v1' to your cms and lms private.py environment.
#. In your terminal, navigate to your
#. In your terminal, run the frontend-app-course-authoring MFE by navigating to the devstack folder and running large-u .
Copy link
Member

Choose a reason for hiding this comment

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

Do you mean make dev.up.large-and-slow?

README.rst Outdated
#. In your terminal, run the frontend-app-course-authoring MFE by navigating to the devstack folder and running large-u .
#. Ensure that the exams IDA is enabled for your course. In your LMS' django admin's course waffle flag table (http://localhost:18000/admin/waffle_utils/waffleflagcourseoverridemodel/) add an entry:
#. Enter course_apps.exams_ida in the Waffle Flag field
#. Enter the course ID for the course ID field (e.g. something like course-v1:edX+COURSENAME+T2023
Copy link
Member

Choose a reason for hiding this comment

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

Nit.

Suggested change
#. Enter the course ID for the course ID field (e.g. something like course-v1:edX+COURSENAME+T2023
#. Enter the course ID for the course ID field (e.g. something like course-v1:edX+COURSENAME+T2023)

README.rst Outdated
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In order to use edx-exams as the exams backend for your courses (and to get it to wor in general), you will also need to complete the following configuration steps:

#. Add EXAMS_SERVICE_URL = 'http://host.docker.internal:18740/api/v1' to your cms and lms private.py environment.
Copy link
Member

Choose a reason for hiding this comment

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

It can be helpful to add a little code formatting to isolate settings and code from the instruction.

Suggested change
#. Add EXAMS_SERVICE_URL = 'http://host.docker.internal:18740/api/v1' to your cms and lms private.py environment.
#. Add ``EXAMS_SERVICE_URL = 'http://host.docker.internal:18740/api/v1'`` to your cms and lms private.py environment.

README.rst Outdated
@@ -59,6 +59,36 @@ Devstack Set Up

You can use the make targets defined in the ``Makefile`` to interact with the running ``edx-exams`` Docker containers.

LTI Configuration for Local Development
Copy link
Member

Choose a reason for hiding this comment

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

Very nice to have these instructions here now!

Are these meant to replace the Local Development and LTI Configuration Confluence page? If so, it would be good to leave a note on that Confluence page referring to this page and to remove the Setting up an exam and proctoring tool section at the bottom of this page, which refers to that Confluence document.

If not, disregard!

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd say so. Since this is meant to be seen by Open Edx's community, I believe it makes sense for this public README to be the source of truth instead of our internal doc. I'll make sure to do that link once this is merged.

Comment on lines +6 to +7
- `Configuring a Proctoring Provider <https://edx.readthedocs.io/projects/edx-partner-course-staff/en/latest/proctored_exams/proctored_enabling.html#configuring-proctoring-provider>`_
- `Creating a Proctored Exam <https://edx.readthedocs.io/projects/edx-partner-course-staff/en/latest/proctored_exams/pt_create.html#creating-a-proctored-exam>`_
Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. So it's like reference materials? My bad - got confused!

#. As your non-staff user in your current window, and enroll in the course where you have set up your proctored exam
#. In an incognito window, login to your staff account and visit your Support Tools page and search in the email for your non-staff user. You should now see the option to change the enrollment track of that user. Change the enrollment track of the user to ""verified"" (choose any reason when prompted, it doesn't matter)
#. In that same incognito, still signed in as staff, navigate to the Instructor dashboard
#. Find or create a test course within Stage with proctoring enabled and an LTI proctoring provider chosen, certificates are enabled, and where the exams IDA waffle flag is enabled
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, totally agree with linking out. I didn't mean to suggest we should duplicate - just that I felt like some information was missing, which can definitely be added by referencing other documentation, like "Set up a proctored exam: ."

docs/test_plan/proctored_exam.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@zacharis278 zacharis278 left a comment

Choose a reason for hiding this comment

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

I'd say we're looking good. One quick thing I noticed, there's a parent readme for this folder that has a link to the other sets of tests. Can you add a link there? https://github.com/edx/edx-exams/tree/main/docs/test_plan

@ilee2u ilee2u merged commit c54428a into main Nov 9, 2023
7 checks passed
@ilee2u ilee2u deleted the ilee2u/proctored_test_docs branch November 9, 2023 20:33
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.

4 participants