Skip to content

Commit

Permalink
Fix versions merging
Browse files Browse the repository at this point in the history
  • Loading branch information
ehmicky committed Jan 9, 2022
1 parent 2c2fd9a commit 8f2546d
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 14 deletions.
3 changes: 2 additions & 1 deletion src/combination/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,12 @@ const getCombinationsProduct = function (tasks, inputsList, system) {
}

const systemDimensions = mapObj(system, getSystemDimension)
return tasks.map(({ id, taskPath, runner }) => ({
return tasks.map(({ id, taskPath, runner: { versions, ...runner } }) => ({
dimensions: { task: { id }, runner, ...systemDimensions },
taskPath,
inputsList,
stats: {},
versions,
}))
}

Expand Down
4 changes: 2 additions & 2 deletions src/history/serialize/compress.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ const compressSystem = function ([
}
}

const compressCombination = function ({ dimensions, stats }) {
const compressCombination = function ({ dimensions, versions, stats }) {
const dimensionsA = mapObj(dimensions, compressDimension)
const statsA = compressStats(stats)
return { dimensions: dimensionsA, stats: statsA }
return { dimensions: dimensionsA, versions, stats: statsA }
}

const compressDimension = function (dimension, { id }) {
Expand Down
7 changes: 5 additions & 2 deletions src/history/serialize/decompress.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@ export const decompressRawResult = function ({
return { id, subId, timestamp, combinations: combinationsA }
}

const decompressCombination = function ({ dimensions, stats }, system) {
const decompressCombination = function (
{ dimensions, stats, versions },
system,
) {
const dimensionsA = mapObj(dimensions, decompressDimension)
const statsA = decompressStats(stats)
return { dimensions: dimensionsA, stats: statsA, system }
return { dimensions: dimensionsA, stats: statsA, system, versions }
}

const decompressDimension = function (dimension, id) {
Expand Down
9 changes: 7 additions & 2 deletions src/run/normalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ export const normalizeNewResult = function (newResult) {
return { ...newResult, combinations }
}

const normalizeNewCombination = function ({ dimensions, stats, system }) {
const normalizeNewCombination = function ({
dimensions,
stats,
system,
versions,
}) {
const dimensionsA = mapObj(dimensions, getIdProp)
return { dimensions: dimensionsA, stats, system }
return { dimensions: dimensionsA, stats, system, versions }
}

const getIdProp = function (propName, { id }) {
Expand Down
13 changes: 6 additions & 7 deletions src/top/system/merge.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import mapObj from 'map-obj'
import { hasPrefix, removePrefix } from '../../combination/prefix.js'
import { groupBy } from '../../utils/group.js'

import { mergeVersions } from './versions/merge.js'

// A result can only have a single `system`.
// - However, when merging results, this becomes several `systems`.
// - We handle this by making systems combination-specific, set to
Expand All @@ -22,10 +24,10 @@ export const mergeSystems = function ({ combinations }) {
return systemsGroups.map(mergeSystemsGroup)
}

const getCombinationSystem = function ({ dimensions, system }) {
const getCombinationSystem = function ({ dimensions, system, versions }) {
const dimensionsA = filterObj(dimensions, isSystemDimension)
const dimensionsB = mapObj(dimensionsA, removeSystemPrefix)
return { ...system, dimensions: dimensionsB }
return { ...system, versions, dimensions: dimensionsB }
}

const isSystemDimension = function (dimensionName) {
Expand Down Expand Up @@ -57,9 +59,6 @@ const mergeSystemsGroup = function ([mostRecentSystem, ...previousSystems]) {
// `git` and `machine` properties should not be deeply merged since their
// properties relate to each other. However, `versions` should.
const mergeSystemsPair = function (system, previousSystem) {
return {
...previousSystem,
...system,
versions: { ...previousSystem.versions, ...system.versions },
}
const versions = mergeVersions(system.versions, previousSystem.versions)
return { ...previousSystem, ...system, versions }
}
40 changes: 40 additions & 0 deletions src/top/system/versions/merge.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import mapObj from 'map-obj'

// Merge the `versions` of either:
// - The same result, but different runners or runnerConfig variations
// - Two results with same ids, with same or different runners or runnerConfig
// variations
// Since each combination might have different versions due to different runners
// and results, but are all reported in the merged result, we report all values
// as a concatenated list.
export const mergeVersions = function (versions, previousVersions) {
return mapObj({ ...versions, ...previousVersions }, (propName) =>
mergeVersionsProp(versions, previousVersions, propName),
)
}

const mergeVersionsProp = function (versions, previousVersions, propName) {
const value = mergeVersionsValue(
versions[propName],
previousVersions[propName],
)
return [propName, value]
}

const mergeVersionsValue = function (values, previousValue) {
if (values === undefined) {
return previousValue
}

if (previousValue === undefined) {
return values
}

const valuesArray = values.split(VERSIONS_VALUE_SEPARATOR)
return valuesArray.includes(previousValue)
? values
: // eslint-disable-next-line fp/no-mutating-methods
[...valuesArray, previousValue].sort().join(VERSIONS_VALUE_SEPARATOR)
}

const VERSIONS_VALUE_SEPARATOR = ', '

0 comments on commit 8f2546d

Please sign in to comment.