From c474991d24176c96c941b1c51687e1eb89e1e85e Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Thu, 9 Nov 2023 17:05:38 -0500 Subject: [PATCH] fix(shared-data): Fix the pipette schemas, tests (#13949) The tests that were supposed to ajv the pipette definitions weren't set up properly, and the tests that were supposed to test that the tests were set up properly weren't set up properly, so we weren't testing the schemas so the schemas were wrong. Fix the schema-test setup, the test-setup-schema-test setup, and the schema. --- .../js/__tests__/pipetteSchemaV2.test.ts | 35 ++--- .../schemas/2/pipettePropertiesSchema.json | 129 ++++++++++-------- .../tests/pipette/test_validate_schema.py | 1 + 3 files changed, 85 insertions(+), 80 deletions(-) diff --git a/shared-data/js/__tests__/pipetteSchemaV2.test.ts b/shared-data/js/__tests__/pipetteSchemaV2.test.ts index ae85233ab8c..66845b9cab6 100644 --- a/shared-data/js/__tests__/pipetteSchemaV2.test.ts +++ b/shared-data/js/__tests__/pipetteSchemaV2.test.ts @@ -13,12 +13,12 @@ const allGeometryDefinitions = path.join( const allGeneralDefinitions = path.join( __dirname, - '../../labware/definitions/2/general/**/**/*.json' + '../../pipette/definitions/2/general/**/**/*.json' ) const allLiquidDefinitions = path.join( __dirname, - '../../labware/definitions/2/liquid/**/**/*.json' + '../../pipette/definitions/2/liquid/**/**/*.json' ) const ajv = new Ajv({ allErrors: true, jsonPointers: true }) @@ -29,11 +29,8 @@ const validateGeneralSpecs = ajv.compile(generalSpecsSchema) describe('test schema against all liquid specs definitions', () => { const liquidPaths = glob.sync(allLiquidDefinitions) - - beforeAll(() => { - // Make sure definitions path didn't break, which would give you false positives - expect(liquidPaths.length).toBeGreaterThan(0) - }) + // Make sure definitions path didn't break, which would give you false positives + expect(liquidPaths.length).toBeGreaterThan(0) liquidPaths.forEach(liquidPath => { const liquidDef = require(liquidPath) @@ -45,22 +42,24 @@ describe('test schema against all liquid specs definitions', () => { expect(valid).toBe(true) }) - it(`parent dir matches pipette model: ${liquidPath}`, () => { - expect(['p10', 'p20', 'p50', 'p300', 'p1000']).toContain( + it(`parent dir matches a liquid class: ${liquidPath}`, () => { + expect(['default', 'lowVolumeDefault']).toContain( path.basename(path.dirname(liquidPath)) ) }) + + it(`second parent dir matches pipette model: ${liquidPath}`, () => { + expect(['p10', 'p20', 'p50', 'p300', 'p1000']).toContain( + path.basename(path.dirname(path.dirname(liquidPath))) + ) + }) }) }) describe('test schema against all geometry specs definitions', () => { const geometryPaths = glob.sync(allGeometryDefinitions) - - beforeAll(() => { - // Make sure definitions path didn't break, which would give you false positives - expect(geometryPaths.length).toBeGreaterThan(0) - }) - + // Make sure definitions path didn't break, which would give you false positives + expect(geometryPaths.length).toBeGreaterThan(0) geometryPaths.forEach(geometryPath => { const geometryDef = require(geometryPath) const geometryParentDir = path.dirname(geometryPath) @@ -88,11 +87,7 @@ describe('test schema against all geometry specs definitions', () => { describe('test schema against all general specs definitions', () => { const generalPaths = glob.sync(allGeneralDefinitions) - - beforeAll(() => { - // Make sure definitions path didn't break, which would give you false positives - expect(generalPaths.length).toBeGreaterThan(0) - }) + expect(generalPaths.length).toBeGreaterThan(0) generalPaths.forEach(generalPath => { const generalDef = require(generalPath) diff --git a/shared-data/pipette/schemas/2/pipettePropertiesSchema.json b/shared-data/pipette/schemas/2/pipettePropertiesSchema.json index 810ceade169..d2b2a7c78fa 100644 --- a/shared-data/pipette/schemas/2/pipettePropertiesSchema.json +++ b/shared-data/pipette/schemas/2/pipettePropertiesSchema.json @@ -1,13 +1,13 @@ { "$schema": "http://json-schema.org/draft-07/schema#", - "$id": "opentronsPipetteGeometrySchemaV2", + "$id": "opentronsPipettePropertiesSchemaV2", "definitions": { "channels": { "enum": [1, 8, 96, 384] }, "displayCategory": { "type": "string", - "enum": ["GEN1"] + "enum": ["GEN1", "GEN2", "FLEX"] }, "positiveNumber": { "type": "number", @@ -18,33 +18,6 @@ "minimum": 0.01, "maximum": 2.5 }, - "xyzArray": { - "type": "array", - "description": "Array of 3 numbers, [x, y, z]", - "items": { "type": "number" }, - "minItems": 3, - "maxItems": 3 - }, - "linearEquations": { - "description": "Array containing any number of 3-arrays. Each inner 3-array describes a line segment: [boundary, slope, intercept]. So [1, 2, 3] would mean 'where (next_boundary > x >= 1), y = 2x + 3'", - "type": "array", - "items": { - "type": "array", - "items": { "type": "number" }, - "minItems": 3, - "maxItems": 3 - } - }, - "liquidHandlingSpecs": { - "description": "Object containing linear equations for translating between uL of liquid and mm of plunger travel. There is one linear equation for aspiration and one for dispense", - "type": "object", - "required": ["aspirate", "dispense"], - "additionalProperties": false, - "properties": { - "aspirate": { "$ref": "#/definitions/linearEquations" }, - "dispense": { "$ref": "#/definitions/linearEquations" } - } - }, "editConfigurations": { "type": "object", "description": "Object allowing you to modify a config", @@ -57,17 +30,6 @@ "type": { "type": "string" }, "displayName": { "type": "string" } } - }, - "tipConfigurations": { - "type": "object", - "description": "Object containing configurations specific to tip handling", - "required": ["speed"], - "properties": { - "presses": {}, - "speed": { "$ref": "#/definitions/editConfigurations" }, - "increment": {}, - "distance": {} - } } }, "description": "Version-level pipette specifications, which may vary across different versions of the same pipette", @@ -103,20 +65,43 @@ }, "channels": { "$ref": "#/definitions/channels" }, "partialTipConfigurations": { - "type": "object", "description": "Object containing information on partial tip configurations", - "required": ["partialTipSupported"], - "properties": { - "partialTipSupported": { "type": "boolean" }, - "availableConfigurations": { - "type": "array", - "description": "Array of available configurations", - "items": { - "type": "number", - "enum": [1, 2, 3, 4, 5, 6, 7, 8, 12, 96, 384] + "$oneof": [ + { + "type": "object", + "required": ["partialTipSupported"], + "properties": { + "partialTipSupported": { "type": "boolean", "value": true } + } + }, + { + "type": "object", + "required": [ + "partialTipSupported", + "availableConfigurations", + "perTipPickupCurrent" + ], + "properties": { + "partialTipSupported": { "type": "boolean", "value": false }, + "availableConfigurations": { + "type": "array", + "description": "Array of available configurations", + "items": { + "type": "number", + "enum": [1, 2, 3, 4, 5, 6, 7, 8, 12, 96, 384] + }, + "perTipPickupCurrent": { + "type": "object", + "patternProperties": { + "\\d+": { + "$ref": "#/definitions/positiveNumber" + } + } + } + } } } - } + ] }, "availableSensors": { "type": "object", @@ -142,13 +127,20 @@ }, "plungerPositionsConfigurations": { "type": "object", - "description": "Object containing configurations specific to tip handling", - "required": ["top", "bottom", "blowout", "drop"], - "properties": { - "top": { "$ref": "#/definitions/currentRange" }, - "bottom": {}, - "blowout": { "$ref": "#/definitions/editConfigurations" }, - "drop": {} + "description": "Key positions of the plunger, by liquid configuration", + "required": ["default"], + "patternProperties": { + "\\w+": { + "type": "object", + "description": "Plunger positions for this liquid configuration", + "required": ["top", "bottom", "blowout", "drop"], + "properties": { + "top": { "type": "number" }, + "bottom": { "type": "number" }, + "blowout": { "type": "number" }, + "drop": { "type": "number" } + } + } } }, "plungerMotorConfigurations": { @@ -170,10 +162,27 @@ } }, "pickUpTipConfigurations": { - "$ref": "#/definitions/tipConfigurations" + "type": "object", + "description": "Object containing configurations for picking up tips common to all partial configurations", + "required": ["speed"], + "properties": { + "presses": { "$ref": "#/definitions/positiveNumber" }, + "speed": { "$ref": "#/definitions/positiveNumber" }, + "increment": { "$ref": "#/definitions/positiveNumber" }, + "distance": { "$ref": "#/definitions/positiveNumber" } + } }, "dropTipConfigurations": { - "$ref": "#/definitions/tipConfigurations" + "type": "object", + "description": "Object containing configurations specific to dropping tips", + "required": ["current", "speed"], + "properties": { + "current": { "$ref": "#/definitions/currentRange" }, + "presses": {}, + "speed": { "$ref": "#/definitions/positiveNumber" }, + "increment": {}, + "distance": {} + } }, "displayName": { "type": "string", diff --git a/shared-data/python/tests/pipette/test_validate_schema.py b/shared-data/python/tests/pipette/test_validate_schema.py index 1ac8d111a16..df943dceace 100644 --- a/shared-data/python/tests/pipette/test_validate_schema.py +++ b/shared-data/python/tests/pipette/test_validate_schema.py @@ -18,6 +18,7 @@ def test_check_all_models_are_valid() -> None: "ninety_six_channel": "96", "eight_channel": "multi", } + assert os.listdir(paths_to_validate), "You have a path wrong" for channel_dir in os.listdir(paths_to_validate): for model_dir in os.listdir(paths_to_validate / channel_dir): for version_file in os.listdir(paths_to_validate / channel_dir / model_dir):