Replace history id by retry hash - #903
Conversation
Allure Report Summary
|
Allure perf metricsGenerated at:
Artifacts: allure-perf-metrics |
0140551 to
82cfc14
Compare
| const testCase: TestCase = { | ||
| id, | ||
| allureId, | ||
| id: testCaseHash, |
There was a problem hiding this comment.
The spec describes _fallbackTestCaseId as a migration path when an adapter changes its test id: current testCaseHash may be new, but fallback hash should let us find the old test case/history. In this PR TestCase.id is set to the current testCaseHash, so when the adapter id changes, the product-level test case id changes too. Where do we use _fallbackTestCaseId to attach the new hash back to the old test case/history, or is that intentionally deferred?
| } | ||
|
|
||
| return selectHistoryTestResults(this.#historyPoints, historyIdCandidates); | ||
| return selectHistoryTestResults(this.#historyPoints, [tr.retryHash]); |
There was a problem hiding this comment.
Can we clarify how _fallbackTestCaseId is supposed to affect history lookup? Right now history is selected only by the current retryHash, so if an adapter changes test id from A to B and sends _fallbackTestCaseId=A, the old history under A still won’t be found. Is fallback-based history migration intentionally deferred to a later catalog/index change?
| const name = raw.name || "Unknown test"; | ||
| const testCase = processTestCase(stateData, raw); | ||
| const parameters = convertParameters(raw.parameters); | ||
| const testCaseHash = calculateTestCaseHash(raw.testId, raw.fullName); |
There was a problem hiding this comment.
Current allure-js adapters often emit an already-hashed testCaseId. With this PR we always compute testCaseHash = md5(raw.testId), so old allure-js output becomes double-hashed, while future canonical output likely won’t. Can we add old-vs-new allure-js fixtures or document the accepted mixed-version behavior?
| const indexTestResultsByHistoryId = await requiredEntryData( | ||
| AllureStoreDumpFiles.IndexTestResultsByHistoryId, | ||
| ); | ||
| const indexTestResultsByRetryHash = |
There was a problem hiding this comment.
Can we clarify what compatibility this legacy history-id read gives us? Since restore rebuilds indexes from test results with the new retryHash rules, I’m not sure old history-id keys are actually migrated here?
|
|
||
| const { name } = parameter; | ||
|
|
||
| if (typeof name !== "string" || name.length === 0 || parameter.excluded === true) { |
There was a problem hiding this comment.
We calculate parametersHash from raw params on first ingest, but after restore we calculate it from converted TestResult.parameters. Are those guaranteed to produce the same hash for null, hidden/masked, excluded, malformed, and duplicate params? A dump/restore regression test for these cases would help.
82cfc14 to
f739a5d
Compare
|
I went over the code but with a very limited experience on working with this codebase can't really say I could thoroughly reviewed it, but once concern that seems to arise if I followed the change correctly, is history continuity after this change. In I believe similar issue could be with ALLURE_ID? It was used for old identity but not for new one? |
3 participants
Context
Checklist