Skip to content

Commit

Permalink
fix: use correct test number sequence for raising github issues (#1128)
Browse files Browse the repository at this point in the history
* Pass sequence number to createIssueLink

* Calculate sequence number in test plan report

* Use sequence number for issue search

* Jsdoc comment for createIssueLink and getIssueLink utility methods

* Update client/components/TestRun/index.jsx

Co-authored-by: Howard Edwards <[email protected]>

---------

Co-authored-by: Howard Edwards <[email protected]>
  • Loading branch information
stalgiag and howard-e authored Jun 20, 2024
1 parent 261e108 commit 70270e3
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ const CandidateTestPlanRun = () => {
versionString: testPlanVersion.versionString,
testTitle: currentTest.title,
testRowNumber: currentTest.rowNumber,
testSequenceNumber: currentTest.seq,
testRenderedUrl: currentTest.renderedUrl,
atName: testPlanReport.at.name
};
Expand Down Expand Up @@ -464,7 +465,7 @@ const CandidateTestPlanRun = () => {
testPlanTitle: testPlanVersion.title,
versionString:
testPlanVersion.versionString,
testRowNumber: currentTest.rowNumber
testSequenceNumber: currentTest.seq
})}
/>
);
Expand Down
1 change: 1 addition & 0 deletions client/components/Reports/SummarizeTestPlanReport.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ const SummarizeTestPlanReport = ({ testPlanVersion, testPlanReports }) => {
versionString: testPlanVersion.versionString,
testTitle: test.title,
testRowNumber: test.rowNumber,
testSequenceNumber: index + 1,
testRenderedUrl: test.renderedUrl,
atName: testPlanReport.at.name,
atVersionName: testResult.atVersion.name,
Expand Down
1 change: 1 addition & 0 deletions client/components/TestRun/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ const TestRun = () => {
)}`,
testTitle: currentTest.title,
testRowNumber: currentTest.rowNumber,
testSequenceNumber: currentTest.seq,
testRenderedUrl: currentTest.renderedUrl,
atName: testPlanReport.at.name,
browserName: testPlanReport.browser.name,
Expand Down
50 changes: 46 additions & 4 deletions client/utils/createIssueLink.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,36 @@ const atLabelMap = {
NVDA: 'nvda'
};

/**
* Creates a link to open a new issue on the GitHub repository.
*
* @param {Object} options - Options for creating the issue link
* @param {boolean} [options.isCandidateReview=false] - Whether this is a candidate review
* @param {boolean} [options.isCandidateReviewChangesRequested=false] - Whether changes are requested for a candidate review
* @param {string} [options.testPlanDirectory] - The directory of the test plan
* @param {string} [options.testPlanTitle] - The title of the test plan
* @param {string} [options.versionString] - The version string
* @param {string|null} [options.testTitle=null] - The title of the test
* @param {number|null} [options.testSequenceNumber=null] - The sequence number of the test. This is the number displayed to test runners
* @param {number|null} [options.testRowNumber=null] - The row number of the test in aria-at
* @param {string|null} [options.testRenderedUrl=null] - The rendered URL of the test
* @param {string} options.atName - The name of the assistive technology
* @param {string|null} [options.atVersionName=null] - The version name of the assistive technology
* @param {string|null} [options.browserName=null] - The name of the browser
* @param {string|null} [options.browserVersionName=null] - The version name of the browser
* @param {string|null} [options.conflictMarkdown=null] - The conflict markdown
* @param {string|null} [options.reportLink=null] - The link to the report
* @returns {string} The URL for creating a new issue on the GitHub repository
* @throws {Error} If required parameters are missing
*/
const createIssueLink = ({
isCandidateReview = false,
isCandidateReviewChangesRequested = false,
testPlanDirectory,
testPlanTitle,
versionString,
testTitle = null,
testSequenceNumber = null,
testRowNumber = null,
testRenderedUrl = null,
atName,
Expand All @@ -29,7 +52,12 @@ const createIssueLink = ({
throw new Error('Cannot create issue link due to missing parameters');
}

const hasTest = !!(testTitle && testRowNumber && testRenderedUrl);
const hasTest = !!(
testTitle &&
testSequenceNumber &&
testRowNumber &&
testRenderedUrl
);

let title;
if (hasTest) {
Expand All @@ -44,7 +72,7 @@ const createIssueLink = ({

title =
`${titleStart}: "${testTitle}" (${testPlanTitle}, ` +
`Test ${testRowNumber}, ${versionString})`;
`Test ${testSequenceNumber}, ${versionString})`;
} else {
title = `${atName} General Feedback: ${testPlanTitle} ${versionString}`;
}
Expand Down Expand Up @@ -99,6 +127,7 @@ const createIssueLink = ({
atName,
browserName,
testRowNumber,
testSequenceNumber,
isCandidateReview,
isCandidateReviewChangesRequested
});
Expand All @@ -121,14 +150,27 @@ const createIssueLink = ({
);
};

/**
* Returns a link to search for existing issues on the GitHub repository based on the provided parameters.
*
* @param {Object} options - Options for generating the issue search link
* @param {boolean} [options.isCandidateReview=false] - Whether this is a candidate review
* @param {boolean} [options.isCandidateReviewChangesRequested=false] - Whether changes are requested for a candidate review
* @param {string|null} [options.username=null] - The username of the author
* @param {string} options.atName - The name of the assistive technology
* @param {string} options.testPlanTitle - The title of the test plan
* @param {string} options.versionString - The version string
* @param {number|null} [options.testSequenceNumber=null] - The sequence number of the test, this is the test number displayed to test runners
* @returns {string} The URL for searching issues on the GitHub repository
*/
export const getIssueSearchLink = ({
isCandidateReview = false,
isCandidateReviewChangesRequested = false,
username = null,
atName,
testPlanTitle,
versionString,
testRowNumber = null
testSequenceNumber = null
}) => {
let atKey;
if (atName === 'JAWS' || atName === 'NVDA') {
Expand All @@ -146,7 +188,7 @@ export const getIssueSearchLink = ({
username ? `author:${username}` : '',
`label:${atKey}`,
`"${testPlanTitle}"`,
testRowNumber ? `Test ${testRowNumber}` : '',
testSequenceNumber ? `Test ${testSequenceNumber}` : '',
versionString
]
.filter(str => str)
Expand Down

0 comments on commit 70270e3

Please sign in to comment.