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

E2E test of issue #8785: Undefined when reading getUpdated #8843

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from
+71 −9
Open

E2E test of issue #8785: Undefined when reading getUpdated #8843

Changes from 4 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
3e0ab4d
drafting a cypress test on issue #8785
Dec 17, 2024
ea7317b
Merge branch 'develop' into ov/regression_reading_getUpdated
Dec 18, 2024
577bb06
add middleware
Dec 18, 2024
f45e810
fix closure, draft test
Dec 18, 2024
351a81a
wip
Dec 19, 2024
2741805
save job and validate error
Dec 20, 2024
04d23c5
Merge branch 'develop' into ov/regression_reading_getUpdated
Dec 20, 2024
3099ab4
add more venv folders
Dec 20, 2024
b0de39c
stub response without handler, check response
Dec 23, 2024
595d104
Merge branch 'develop' into ov/regression_reading_getUpdated
Dec 23, 2024
5d67f16
removve useless function
Dec 23, 2024
2b599a5
rename function
archibald1418 Dec 23, 2024
120ef97
move /.*env* lower to package section, add *venv*/ for Python packages
Dec 23, 2024
31eab5c
rename deleteFrame function, rename ctx, add assertion on response body
Dec 23, 2024
b90538c
factored out checking of 'Delete frames' checkbox into a support cmd,…
Dec 23, 2024
adfe755
renam issue_8785 test file
Dec 23, 2024
a1bd735
spelling error in unit name
Dec 23, 2024
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
77 changes: 77 additions & 0 deletions ...ress/e2e/issues_prs2/issue_8785_Cannot_read_properties_of_undefined_reading_getUpdated.js
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets name this issue_8785_update_job_metadata

Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Copyright (C) 2024 CVAT.ai Corporation
//
// SPDX-License-Identifier: MIT

/// <reference types="cypress" />

import { taskName } from '../../support/const';

context('When saving after deleting a frame, job metadata is inconsistent.', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
context('When saving after deleting a frame, job metadata is inconsistent.', () => {
context('The UI remains stable even when the metadata request fails.', () => {

const issueId = '8785';

before(() => {
cy.openTaskJob(taskName);
cy.goToNextFrame(4);
});

describe(`Testing issue ${issueId}`, () => {
it('Crash on Save job. Save again.', () => {
// cy.deleteFrame(); // FIXME: does normal saving with 200, better to just push a button
function clickDelete() {
cy.get('.cvat-player-delete-frame').click();
cy.get('.cvat-modal-delete-frame').within(() => {
cy.contains('button', 'Delete').click();
});
}
function clickSave() {
cy.get('button').contains('Save').click({ force: true });
cy.get('button').contains('Save').trigger('mouseout');
}
function middleware() {
let calls = 0;
const badResponseStub = { statusCode: 502, body: 'Network error' };
function handle(req, res) {
if (calls === 0) {
console.log(calls);
calls++;
res.send(badResponseStub);
} else {
console.log(calls);
req.continue({ statusCode: 200, body: 'OK' });
}
}
return handle;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve middleware implementation

The middleware implementation could benefit from several improvements:

  1. Add error handling for edge cases
  2. Use typed response objects
  3. Add logging for debugging purposes
 function middleware() {
     let calls = 0;
-    const badResponseStub = { statusCode: 502, body: 'Network error' };
+    const badResponseStub = {
+        statusCode: 502,
+        body: { error: 'Network error' },
+        headers: { 'content-type': 'application/json' },
+    };
     function handle(req, res) {
+        try {
             if (calls === 0) {
                 calls++;
                 res.send(badResponseStub);
             } else {
                 req.continue({ statusCode: 200, body: 'OK' });
             }
+        } catch (error) {
+            cy.log(`Middleware error: ${error.message}`);
+            throw error;
+        }
     }
     return handle;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function middleware() {
let calls = 0;
const badResponseStub = { statusCode: 502, body: 'Network error' };
function handle(req, res) {
if (calls === 0) {
console.log(calls);
calls++;
res.send(badResponseStub);
} else {
console.log(calls);
req.continue({ statusCode: 200, body: 'OK' });
}
}
return handle;
}
function middleware() {
let calls = 0;
const badResponseStub = {
statusCode: 502,
body: { error: 'Network error' },
headers: { 'content-type': 'application/json' },
};
function handle(req, res) {
try {
if (calls === 0) {
calls++;
res.send(badResponseStub);
} else {
req.continue({ statusCode: 200, body: 'OK' });
}
} catch (error) {
cy.log(`Middleware error: ${error.message}`);
throw error;
}
}
return handle;
}

cy.intercept('PATCH', '/api/jobs/**/data/meta**', middleware()).as('patchMeta');
clickDelete();
clickSave();
cy.contains('button', 'Restore').should('be.visible');
cy.wait('@patchMeta').its('response.body').should('eq', 502);
cy.wait('@patchMeta').its('response.body').should('eq', 200);

// TODO: refactor saveJob with necessary status

// Check that frame is deleted
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add missing frame deletion assertions

The comment indicates a need for frame deletion verification, but no assertions are implemented. Consider adding appropriate checks.

// Add these assertions after line 54
cy.get('.cvat-player-frame-selector').should('have.value', TEST_FRAME_NUMBER - 1);
cy.get('.cvat-player-restore-frame').should('exist');


/**
* FIXME: this just asserts 502
*
* Here you have an example job endpoint response stubbing
* Response is then intercepted and stubbed with 502 status code
*
* flow is like this: press a button directly + intercept
*
* Cypress works worse with double intercepts
* Intercepting saveJob not gonna work
* since it already does an intercept of the same request
*
* */

// Send bad PATCH
cy.saveJob('PATCH', 502);

// Send again
cy.saveJob('PATCH', 200);
});
});
});
Loading