Add the profile urls to performance alerts only if the profile artifacts are available (#7601)

* Add the profile urls to performance alerts only if the profile artifacts are available

* Add a test to make sure we don't output the profile links when there is no profile artifact
This commit is contained in:
Nazım Can Altınova 2022-12-21 20:47:11 +01:00 коммит произвёл GitHub
Родитель 4238c2c1fc
Коммит cdf685ed4f
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
2 изменённых файлов: 68 добавлений и 2 удалений

Просмотреть файл

@ -15,6 +15,11 @@ import prevJoblistWithoutVideoResultsOne from '../mock/alerts_with_videos/prev_j
import prevJoblistWithoutVideoResultsTwo from '../mock/alerts_with_videos/prev_joblist_without_video_results_page_2'; import prevJoblistWithoutVideoResultsTwo from '../mock/alerts_with_videos/prev_joblist_without_video_results_page_2';
import repos from '../mock/repositories'; import repos from '../mock/repositories';
const browsertimeProfileUrls = [
'https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/c6nAPFniThOiaGvDT2DNCw/runs/0/artifacts/public/test_info/profile_google-sheets.zip',
'https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/LUjQR22tRp6J4wXLHhYk8g/runs/0/artifacts/public/test_info/profile_google-sheets.zip',
];
describe('BrowsertimeAlertsExtraData', () => { describe('BrowsertimeAlertsExtraData', () => {
afterEach(() => { afterEach(() => {
fetchMock.reset(); fetchMock.reset();
@ -40,6 +45,10 @@ describe('BrowsertimeAlertsExtraData', () => {
); );
// Returns the autoland repo. // Returns the autoland repo.
fetchMock.mock(`/api/repository/`, repos); fetchMock.mock(`/api/repository/`, repos);
for (const profileUrl of browsertimeProfileUrls) {
fetchMock.mock(profileUrl, { ok: true });
}
}); });
test('should return alerts with browsertime results links', async () => { test('should return alerts with browsertime results links', async () => {
@ -175,4 +184,35 @@ describe('BrowsertimeAlertsExtraData', () => {
}); });
}); });
}); });
describe('File bug url when alerts do not contain profile artifacts', () => {
beforeEach(() => {
for (const profileUrl of browsertimeProfileUrls) {
// Returning `ok: false` to imitate a 404.
fetchMock.mock(profileUrl, { ok: false });
}
});
test('should return alerts without profiler links', async () => {
const alertSummaryNonBrowsertime = cloneDeep(
testAlertSummaryNonBrowsertime,
);
const alertsWithoutVideos = new BrowsertimeAlertsExtraData(
alertSummaryNonBrowsertime,
[{ id: 13, name: 'browsertime' }],
);
const alerts = await alertsWithoutVideos.enrichAndRetrieveAlerts();
expect(alerts).toStrictEqual([]);
alertSummaryNonBrowsertime.alerts.forEach((alert) => {
if (alertsWithoutVideos.shouldHaveVideoLinks(alert)) {
return;
}
// There should be no profile links because the profile artifacts
// are not found.
expect(alert.profile_url).toBeUndefined();
expect(alert.prev_profile_url).toBeUndefined();
});
});
});
}); });

Просмотреть файл

@ -94,7 +94,7 @@ export default class BrowsertimeAlertsExtraData {
} }
async enrichWithProfileLinks(alertSummary, repo, jobList) { async enrichWithProfileLinks(alertSummary, repo, jobList) {
alertSummary.alerts.forEach((alert) => { let alertLinks = alertSummary.alerts.map((alert) => {
const job = jobList.data.find( const job = jobList.data.find(
(j) => (j) =>
j.searchStr.includes(alert.series_signature.suite) && j.searchStr.includes(alert.series_signature.suite) &&
@ -112,6 +112,32 @@ export default class BrowsertimeAlertsExtraData {
artifactPath: `public/test_info/profile_${suite}.zip`, artifactPath: `public/test_info/profile_${suite}.zip`,
}); });
// We check the artifacts with `method: 'HEAD'` which means that it
// doesn't download the whole file and only gets the headers. We can see
// if artifact is available or not using the headers.
const promise = fetch(url, {
method: 'HEAD',
});
return { alert, job, promise, url };
}
return null;
});
// Keep only the alerts that have a job and url. Filter out the nulls.
alertLinks = alertLinks.filter((a) => a);
// We don't await in the loop above because we don't want to block the loop
// on each iteration. Promise.all will properly parallelize the requests.
const promiseResults = await Promise.all(alertLinks.map((a) => a.promise));
// Now we know if the artifacts are available or not, we can add the links
// to the alerts.
for (let i = 0; i < alertLinks.length; i++) {
const { alert, job, url } = alertLinks[i];
const promiseResult = promiseResults[i];
if (promiseResult.ok) {
// Add profile urls to the alert only if the artifact is present.
if (job.push_revision === alertSummary.revision) { if (job.push_revision === alertSummary.revision) {
alert.profile_url = url; alert.profile_url = url;
} }
@ -120,6 +146,6 @@ export default class BrowsertimeAlertsExtraData {
alert.prev_profile_url = url; alert.prev_profile_url = url;
} }
} }
}); }
} }
} }