#2180 - Assessment Workflow Enqueuer Scheduler - Start Assessment - E2E tests #2314

andrewsignori-aot · 2023-09-19T00:14:20Z

Create a few E2E tests for the new schedler.
- Should queue an assessment when an active application has only the original assessment and it is in 'Submitted' state. (119 ms)
- Should queue the oldest pending reassessment when an active application has one completed original assessment and multiple submitted reassessments queued. (145 ms)
- Should not queue a 'Submitted' assessment if the another assessment has its status as 'Queued' (99 ms)
- Should not queue a 'Submitted' assessment if the another assessment has its status as 'In progress' (101 ms)
- Should not queue a 'Submitted' assessment if currentAssessment and currentProcessingAssessment are the same already. (96 ms)
Created a method to centralize the mock name creation and allow retrieve the mock to execute tests like toBeCalledWith.

startApplicationAssessmentQueueMock = app.get(
    getQueueProviderName(QueueNames.StartApplicationAssessment),
);

expect(startApplicationAssessmentQueueMock.add).toBeCalledWith(queueData);

sonarqubecloud · 2023-09-19T16:13:13Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

github-actions · 2023-09-19T16:14:37Z

Backend Unit Tests Coverage Report

Totals
Statements:	17.56% ( 2198 / 12520 )
Methods:	8.09% ( 128 / 1583 )
Lines:	20.32% ( 1928 / 9486 )
Branches:	9.79% ( 142 / 1451 )

github-actions · 2023-09-19T16:15:28Z

E2E Workflow Workers Coverage Report

Totals
Statements:	46.73% ( 300 / 642 )
Methods:	40% ( 32 / 80 )
Lines:	51.02% ( 251 / 492 )
Branches:	24.29% ( 17 / 70 )

github-actions · 2023-09-19T16:16:47Z

E2E Queue Consumers Coverage Report

Totals
Statements:	74.31% ( 483 / 650 )
Methods:	66.67% ( 54 / 81 )
Lines:	76.4% ( 424 / 555 )
Branches:	35.71% ( 5 / 14 )

github-actions · 2023-09-19T16:25:11Z

E2E SIMS API Coverage Report

Totals
Statements:	54.48% ( 3995 / 7333 )
Methods:	51.31% ( 488 / 951 )
Lines:	59.3% ( 3245 / 5472 )
Branches:	28.79% ( 262 / 910 )

...rc/processors/schedulers/workflow/_tests_/assessment-workflow-enqueuer.scheduler.e2e-spec.ts

ann-aot · 2023-09-19T20:15:08Z

...rc/processors/schedulers/workflow/_tests_/assessment-workflow-enqueuer.scheduler.e2e-spec.ts

+      // Arrange
+
+      // Application submitted with original assessment.
+      const application = await createDefaultApplication();


Trying to understand the purpose of the new method? won't it be able to achieve by saveFakeApplicationDisbursements ?

It is just a centralization inside the same TC file. There are positive and negative scenarios using the same method which makes it better to assume that, when there is a negative test it would be more accurate because the positive is already using the same method.

These test cases are also targeting student assessments, not disbursements. Can you please point out the benefit of using the saveFakeApplicationDisbursements?
@andrepestana-aot FYI

ann-aot · 2023-09-19T20:17:35Z

...rc/processors/schedulers/workflow/_tests_/assessment-workflow-enqueuer.scheduler.e2e-spec.ts

+     * the provided student assessment.
+     */
+    const createDefaultApplication = async (
+      assessmentStatus?: StudentAssessmentStatus,


options?: with partial<>?

This is a simple helper method inside a TC file itself and not a shared factory. Should we follow the above-mentioned suggestion for such method?

I don't have a strong feeling, I am good with both, unless the options are big

It is a local method and it is not intended to be shared and the comments state its purpose.
If there is not a strong feeling I will keep the method as it is and state again, I never saw a reason for local methods to follow the patterns used on factories, options yes, we use it all around, "options?: with partial<>" is something to be enforced from factories. Hope that it is ok.

ann-aot · 2023-09-19T20:31:25Z

...rc/processors/schedulers/workflow/_tests_/assessment-workflow-enqueuer.scheduler.e2e-spec.ts

+        application,
+        auditUser,
+      });
+      oldestAssessment.triggerType = AssessmentTriggerType.OfferingChange;


ann-aot · 2023-09-19T20:36:11Z

...rc/processors/schedulers/workflow/_tests_/assessment-workflow-enqueuer.scheduler.e2e-spec.ts

+    [
+      StudentAssessmentStatus.Queued,
+      StudentAssessmentStatus.InProgress,
+    ].forEach((status) => {


ann-aot

LGTM @andrewsignori-aot 👍 just a minor comment and a question

andrepestana-aot

Looks good but I agree with Ann on #2314 (review)
Please take a look. Thanks.

guru-aot

LGTM, nice work @andrewsignori-aot

ann-aot

Nice work @andrewsignori-aot 👍

andrepestana-aot

Looks good.

dheepak-aot · 2023-09-19T22:52:18Z

...rc/processors/schedulers/workflow/_tests_/assessment-workflow-enqueuer.scheduler.e2e-spec.ts

+    [
+      StudentAssessmentStatus.Queued,
+      StudentAssessmentStatus.InProgress,
+    ].forEach((status) => {


dheepak-aot · 2023-09-19T22:57:02Z

...rc/processors/schedulers/workflow/_tests_/assessment-workflow-enqueuer.scheduler.e2e-spec.ts

+
+        // Application submitted with original assessment.
+        const application = await createDefaultApplication(status);
+        const submittedAssessment = createFakeStudentAssessment({


Should we not update application.currentAssessment = submittedAssessment ?

That operation happens inside the factory already used in the method createDefaultApplication.

dheepak-aot · 2023-09-19T22:59:50Z

...rc/processors/schedulers/workflow/_tests_/assessment-workflow-enqueuer.scheduler.e2e-spec.ts

+      });
+    });
+
+    it(`Should not queue a '${StudentAssessmentStatus.Submitted}' assessment if currentAssessment and currentProcessingAssessment are the same already.`, async () => {


I am wondering how currentAssessment and currentProcessing assessment could be same when assessment status is submitted?

In the real world, they would not have it. This condition is to force the specific SQL condition. That is what the below comment is stating.
"// Force currentAssessment and currentProcessingAssessment to be the same to test the SQL condition."

dheepak-aot · 2023-09-19T23:01:01Z

Great work. One clarification and one minor question.

sh16011993

Nice Work @andrewsignori-aot 👍

dheepak-aot

Looks good 👍

E2E tests for assessment queue

ba5890e

andrewsignori-aot added Queue Consumers E2E/Unit tests labels Sep 19, 2023

andrewsignori-aot self-assigned this Sep 19, 2023

andrewsignori-aot added 2 commits September 19, 2023 09:07

Centralize load data

3bb1ef5

Added one more E2E

170d6d9

andrewsignori-aot marked this pull request as ready for review September 19, 2023 16:12

andrewsignori-aot requested review from sh16011993, dheepak-aot, guru-aot, ann-aot, andrepestana-aot and JasonCTang September 19, 2023 16:13

sh16011993 reviewed Sep 19, 2023

View reviewed changes

...rc/processors/schedulers/workflow/_tests_/assessment-workflow-enqueuer.scheduler.e2e-spec.ts Show resolved Hide resolved

ann-aot reviewed Sep 19, 2023

View reviewed changes

andrepestana-aot reviewed Sep 19, 2023

View reviewed changes

guru-aot approved these changes Sep 19, 2023

View reviewed changes

ann-aot approved these changes Sep 19, 2023

View reviewed changes

andrepestana-aot approved these changes Sep 19, 2023

View reviewed changes

dheepak-aot reviewed Sep 19, 2023

View reviewed changes

sh16011993 approved these changes Sep 20, 2023

View reviewed changes

dheepak-aot approved these changes Sep 20, 2023

View reviewed changes

andrewsignori-aot changed the title ~~#2180 - Assessment Workflow Enqueuer Scheduler - Start Assessment- E2E tests~~ #2180 - Assessment Workflow Enqueuer Scheduler - Start Assessment - E2E tests Sep 20, 2023

andrewsignori-aot merged commit 17cf172 into main Sep 20, 2023

andrewsignori-aot deleted the feature/#2180-assessment-workflow-enqueuer-e2es branch September 20, 2023 17:25

andrewsignori-aot temporarily deployed to DEV September 20, 2023 17:41 — with GitHub Actions Inactive

andrewsignori-aot temporarily deployed to DEV September 20, 2023 17:43 — with GitHub Actions Inactive

andrewsignori-aot temporarily deployed to DEV September 20, 2023 17:45 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#2180 - Assessment Workflow Enqueuer Scheduler - Start Assessment - E2E tests #2314

#2180 - Assessment Workflow Enqueuer Scheduler - Start Assessment - E2E tests #2314

andrewsignori-aot commented Sep 19, 2023 •

edited

Loading

sonarqubecloud bot commented Sep 19, 2023

github-actions bot commented Sep 19, 2023

github-actions bot commented Sep 19, 2023

github-actions bot commented Sep 19, 2023

github-actions bot commented Sep 19, 2023

ann-aot Sep 19, 2023

andrewsignori-aot Sep 19, 2023

andrewsignori-aot Sep 19, 2023

ann-aot Sep 19, 2023 •

edited

Loading

andrewsignori-aot Sep 19, 2023

ann-aot Sep 19, 2023 •

edited

Loading

andrewsignori-aot Sep 19, 2023

ann-aot Sep 19, 2023

ann-aot Sep 19, 2023

ann-aot left a comment

andrepestana-aot left a comment

guru-aot left a comment

ann-aot left a comment

andrepestana-aot left a comment

dheepak-aot Sep 19, 2023

dheepak-aot Sep 19, 2023

andrewsignori-aot Sep 20, 2023

dheepak-aot Sep 19, 2023

andrewsignori-aot Sep 20, 2023

dheepak-aot commented Sep 19, 2023

sh16011993 left a comment

dheepak-aot left a comment

#2180 - Assessment Workflow Enqueuer Scheduler - Start Assessment - E2E tests #2314

#2180 - Assessment Workflow Enqueuer Scheduler - Start Assessment - E2E tests #2314

Conversation

andrewsignori-aot commented Sep 19, 2023 • edited Loading

sonarqubecloud bot commented Sep 19, 2023

github-actions bot commented Sep 19, 2023

Backend Unit Tests Coverage Report

github-actions bot commented Sep 19, 2023

E2E Workflow Workers Coverage Report

github-actions bot commented Sep 19, 2023

E2E Queue Consumers Coverage Report

github-actions bot commented Sep 19, 2023

E2E SIMS API Coverage Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ann-aot Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ann-aot Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ann-aot left a comment

Choose a reason for hiding this comment

andrepestana-aot left a comment

Choose a reason for hiding this comment

guru-aot left a comment

Choose a reason for hiding this comment

ann-aot left a comment

Choose a reason for hiding this comment

andrepestana-aot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dheepak-aot commented Sep 19, 2023

sh16011993 left a comment

Choose a reason for hiding this comment

dheepak-aot left a comment

Choose a reason for hiding this comment

andrewsignori-aot commented Sep 19, 2023 •

edited

Loading

ann-aot Sep 19, 2023 •

edited

Loading

ann-aot Sep 19, 2023 •

edited

Loading