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

Address edge case scenarios during the updating of TestPlanVersion phase with old TestPlanVersion results #771

Merged
merged 7 commits into from
Sep 28, 2023
Merged

Address edge case scenarios during the updating of TestPlanVersion phase with old TestPlanVersion results #771

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9c46c10
Revise deprecation process when copying results from earlier version …
howard-e Sep 6, 2023
c6f2ccb
Additional edge case management for updatePhaseResolver during copyin…
howard-e Sep 7, 2023
b7a34b7
Address edge cases during phase update process when using old results
howard-e Sep 11, 2023
1ebb5bc
Update tests to include updatePhase process
howard-e Sep 11, 2023
3b9c22c
Extend default timeout
howard-e Sep 11, 2023
fc1ce6a
Make db docs consistent between test and dev
alflennik Sep 26, 2023
7ad790c
Update database.md formatting
howard-e Sep 26, 2023
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/runtest.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ jobs:
yarn sequelize:test db:migrate
yarn sequelize:test db:seed:all
yarn workspace server db-import-tests:test -c ${IMPORT_ARIA_AT_TESTS_COMMIT_1}
yarn workspace server db-import-tests:test -c ${IMPORT_ARIA_AT_TESTS_COMMIT_2}
yarn workspace server db-import-tests:test
yarn workspace server db-populate-sample-data:test
- name: test
Expand Down
55 changes: 18 additions & 37 deletions client/components/DataManagement/DataManagementRow/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,18 +636,14 @@ const DataManagementRow = ({
}

let coveredReports = [];
let finalReportFound = false;

latestVersion.testPlanReports.forEach(testPlanReport => {
const markedFinalAt = testPlanReport.markedFinalAt;
const atName = testPlanReport.at.name;
const browserName = testPlanReport.browser.name;
const value = `${atName}_${browserName}`;

if (markedFinalAt && !coveredReports.includes(value)) {
finalReportFound = true;
if (markedFinalAt && !coveredReports.includes(value))
coveredReports.push(value);
}
});

// Phase is "active"
Expand All @@ -667,38 +663,23 @@ const DataManagementRow = ({
className="advance-button"
variant="secondary"
onClick={async () => {
if (finalReportFound) {
setShowAdvanceModal(true);
setAdvanceModalData({
phase: derivePhaseName(
'CANDIDATE'
),
version: convertDateToString(
latestVersionDate,
'YY.MM.DD'
),
advanceFunc: () => {
setShowAdvanceModal(false);
handleClickUpdateTestPlanVersionPhase(
latestVersion.id,
'CANDIDATE',
testPlanVersionDataToInclude
);
},
coveredReports
});
} else {
setShowThemedModal(true);
setThemedModalTitle(
'Error Updating Test Plan Version Phase'
);
setThemedModalContent(
<>
No reports have been marked
as final.
</>
);
}
setShowAdvanceModal(true);
setAdvanceModalData({
phase: derivePhaseName('CANDIDATE'),
version: convertDateToString(
latestVersionDate,
'YY.MM.DD'
),
advanceFunc: () => {
setShowAdvanceModal(false);
handleClickUpdateTestPlanVersionPhase(
latestVersion.id,
'CANDIDATE',
testPlanVersionDataToInclude
);
},
coveredReports
});
}}
>
Advance to Candidate
Expand Down
3 changes: 2 additions & 1 deletion config/test.env
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ ENVIRONMENT=test
ALLOW_FAKE_ROLE=true
IMPORT_CONFIG=../config/test.env

IMPORT_ARIA_AT_TESTS_COMMIT_1=1aa3b74d24d340362e9f511eae33788d55487d12
IMPORT_ARIA_AT_TESTS_COMMIT_1=5fe7afd82fe51c185b8661276105190a59d47322
IMPORT_ARIA_AT_TESTS_COMMIT_2=1aa3b74d24d340362e9f511eae33788d55487d12

GITHUB_OAUTH_SERVER=http://localhost:4466
GITHUB_GRAPHQL_SERVER=http://localhost:4466
Expand Down
2 changes: 2 additions & 0 deletions docs/database.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ The instructions are similar for the test database, with one extra step:
yarn db-init:test;
yarn sequelize:test db:migrate;
yarn sequelize:test db:seed:all;
yarn workspace server db-import-tests:test -c 5fe7afd82fe51c185b8661276105190a59d47322;
yarn workspace server db-import-tests:test -c 1aa3b74d24d340362e9f511eae33788d55487d12;
yarn workspace server db-import-tests:test;
yarn workspace server db-populate-sample-data:test;
```
Expand Down
175 changes: 157 additions & 18 deletions server/resolvers/TestPlanVersionOperations/updatePhaseResolver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ const { AuthenticationError } = require('apollo-server');
const {
updateTestPlanReport,
getTestPlanReports,
getOrCreateTestPlanReport
getOrCreateTestPlanReport,
removeTestPlanReport
} = require('../../models/services/TestPlanReportService');
const conflictsResolver = require('../TestPlanReport/conflictsResolver');
const finalizedTestResultsResolver = require('../TestPlanReport/finalizedTestResultsResolver');
Expand Down Expand Up @@ -51,6 +52,7 @@ const updatePhaseResolver = async (

let testPlanVersionDataToInclude;
let testPlanReportsDataToIncludeId = [];
let createdTestPlanReportIdsFromOldResults = [];

// The testPlanVersion being updated
const testPlanVersion = await getTestPlanVersionById(testPlanVersionId);
Expand Down Expand Up @@ -117,14 +119,23 @@ const updatePhaseResolver = async (
// between versions
let keptTestIds = {};
for (const testPlanVersionTest of testPlanVersion.tests) {
const testHash = hashTest(testPlanVersionTest);
// Found odd instances of rowNumber being an int instead of being how it
// currently is; imported as a string
// Ensuring proper hashes are done here
const testHash = hashTest({
...testPlanVersionTest,
rowNumber: String(testPlanVersionTest.rowNumber)
});

if (keptTestIds[testHash]) continue;

for (const testPlanVersionDataToIncludeTest of testPlanVersionDataToInclude.tests) {
const testDataToIncludeHash = hashTest(
testPlanVersionDataToIncludeTest
);
const testDataToIncludeHash = hashTest({
...testPlanVersionDataToIncludeTest,
rowNumber: String(
testPlanVersionDataToIncludeTest.rowNumber
)
});

if (testHash === testDataToIncludeHash) {
if (!keptTestIds[testHash])
Expand All @@ -150,7 +161,7 @@ const updatePhaseResolver = async (
// Then this data should be preserved
testResultsToSave[testId] = testResult;
} else {
// TODO: Track which tests cannot be preserved
// TODO: Return information on which tests cannot be preserved
}
});
}
Expand All @@ -163,6 +174,10 @@ const updatePhaseResolver = async (
browserId: testPlanReportDataToInclude.browserId
});

createdTestPlanReportIdsFromOldResults.push(
createdTestPlanReport.id
);

const createdTestPlanRun = await createTestPlanRun({
testerUserId: testPlanRun.testerUserId,
testPlanReportId: createdTestPlanReport.id
Expand Down Expand Up @@ -231,9 +246,88 @@ const updatePhaseResolver = async (
testResults.push(testResultToSave);
}

// Update TestPlanRun test results to be used in metrics evaluation
// afterward
await updateTestPlanRun(createdTestPlanRun.id, {
testResults
});

// Update metrics for TestPlanReport
const { testPlanReport: populatedTestPlanReport } =
await populateData(
{ testPlanReportId: createdTestPlanReport.id },
{ context }
);

const runnableTests = runnableTestsResolver(
populatedTestPlanReport
);
let updateParams = {};

// Mark the report as final if previously was on the TestPlanVersion being
// deprecated
if (testPlanReportDataToInclude.markedFinalAt)
updateParams = { markedFinalAt: new Date() };

// Calculate the metrics (happens if updating to DRAFT)
const conflicts = await conflictsResolver(
populatedTestPlanReport,
null,
context
);

if (conflicts.length > 0) {
// Then no chance to have finalized reports, and means it hasn't been
// marked as final yet
updateParams = {
...updateParams,
metrics: {
...populatedTestPlanReport.metrics,
conflictsCount: conflicts.length
}
};
} else {
const finalizedTestResults =
await finalizedTestResultsResolver(
populatedTestPlanReport,
null,
context
);

if (
!finalizedTestResults ||
!finalizedTestResults.length
) {
// Just update with current { markedFinalAt } if available
updateParams = {
...updateParams,
metrics: {
...populatedTestPlanReport.metrics
}
};
} else {
const metrics = getMetrics({
testPlanReport: {
...populatedTestPlanReport,
finalizedTestResults,
runnableTests
}
});

updateParams = {
...updateParams,
metrics: {
...populatedTestPlanReport.metrics,
...metrics
}
};
}
}

await updateTestPlanReport(
populatedTestPlanReport.id,
updateParams
);
}
}
}
Expand Down Expand Up @@ -262,12 +356,26 @@ const updatePhaseResolver = async (
throw new Error('No test plan reports found.');
}

// If there is at least one report that wasn't created by the old reports then do the exception
// check
if (
!testPlanReports.some(({ markedFinalAt }) => markedFinalAt) &&
(phase === 'CANDIDATE' || phase === 'RECOMMENDED')
testPlanReports.some(
({ id }) => !createdTestPlanReportIdsFromOldResults.includes(id)
)
) {
// Do not update phase if no reports marked as final were found
throw new Error('No reports have been marked as final.');
if (
!testPlanReports.some(({ markedFinalAt }) => markedFinalAt) &&
(phase === 'CANDIDATE' || phase === 'RECOMMENDED')
) {
// Throw away newly created test plan reports if exception was hit
if (createdTestPlanReportIdsFromOldResults.length)
for (const createdTestPlanReportId of createdTestPlanReportIdsFromOldResults) {
await removeTestPlanReport(createdTestPlanReportId);
}

// Do not update phase if no reports marked as final were found
throw new Error('No reports have been marked as final.');
}
}

if (phase === 'CANDIDATE') {
Expand Down Expand Up @@ -304,6 +412,12 @@ const updatePhaseResolver = async (
});

if (missingAtBrowserCombinations.length) {
// Throw away newly created test plan reports if exception was hit
if (createdTestPlanReportIdsFromOldResults.length)
for (const createdTestPlanReportId of createdTestPlanReportIdsFromOldResults) {
await removeTestPlanReport(createdTestPlanReportId);
}

throw new Error(
`Cannot set phase to ${phase.toLowerCase()} because the following` +
` required reports have not been collected or finalized:` +
Expand All @@ -316,42 +430,67 @@ const updatePhaseResolver = async (
const runnableTests = runnableTestsResolver(testPlanReport);
let updateParams = {};

const isReportCreatedFromOldResults =
createdTestPlanReportIdsFromOldResults.includes(testPlanReport.id);

if (phase === 'DRAFT') {
const conflicts = await conflictsResolver(
testPlanReport,
null,
context
);
await updateTestPlanReport(testPlanReport.id, {

updateParams = {
metrics: {
...testPlanReport.metrics,
conflictsCount: conflicts.length
},
markedFinalAt: null
});
}
};

// Nullify markedFinalAt if not using old result
if (!isReportCreatedFromOldResults)
updateParams = { ...updateParams, markedFinalAt: null };

await updateTestPlanReport(testPlanReport.id, updateParams);
}

if (phase === 'CANDIDATE' || phase === 'RECOMMENDED') {
const shouldThrowErrorIfFound =
(phase === 'CANDIDATE' || phase === 'RECOMMENDED') &&
isReportCreatedFromOldResults
? false
: testPlanReport.markedFinalAt;

if (shouldThrowErrorIfFound) {
const conflicts = await conflictsResolver(
testPlanReport,
null,
context
);
if (conflicts.length > 0) {
// Throw away newly created test plan reports if exception was hit
if (createdTestPlanReportIdsFromOldResults.length)
for (const createdTestPlanReportId of createdTestPlanReportIdsFromOldResults) {
await removeTestPlanReport(createdTestPlanReportId);
}

throw new Error(
'Cannot update test plan report due to conflicts'
);
}

const finalizedTestResults = await finalizedTestResultsResolver(
{
...testPlanReport
},
testPlanReport,
null,
context
);

if (!finalizedTestResults || !finalizedTestResults.length) {
// Throw away newly created test plan reports if exception was hit
if (createdTestPlanReportIdsFromOldResults.length)
for (const createdTestPlanReportId of createdTestPlanReportIdsFromOldResults) {
await removeTestPlanReport(createdTestPlanReportId);
}

throw new Error(
'Cannot update test plan report because there are no ' +
'completed test results'
Expand Down
Loading