From 7ba13487fab687d501e9ca77fd2d593a209992ce Mon Sep 17 00:00:00 2001 From: Justin Stayton Date: Thu, 27 Aug 2026 06:44:24 -0400 Subject: [PATCH] Fix tests that assert the wrong thing Each of these tests passed while asserting something other than what its name claimed, so a regression in the code under test wouldn't have failed the suite. Every case was confirmed by a mutation that survived the full suite before this change and fails after it. The page parser's string branch was unreachable: `parse()` validates first, and Joi coerces a numeric string to a number. The operand dates to the initial commit and was carried through the `is` removal mechanically. Removing it leaves coverage at 100%. The validator tests now assert the validated values in the returned map, rather than only that a `Map` came back. That write-back is what turns `filter[x][is]=null` into SQL `is null`. Page write-back still needs an end-to-end assertion. Co-Authored-By: Claude Opus 5 --- src/adapters/base.test.js | 8 ++------ src/parsers/page.js | 5 +---- src/parsers/page.test.js | 2 +- src/validators/adapter.test.js | 27 ++++++++++++++++++--------- src/validators/querier/joi.test.js | 29 +++++++++++++++++++---------- 5 files changed, 41 insertions(+), 30 deletions(-) diff --git a/src/adapters/base.test.js b/src/adapters/base.test.js index 0ac8ab4..11c3994 100644 --- a/src/adapters/base.test.js +++ b/src/adapters/base.test.js @@ -67,10 +67,8 @@ describe('filter', () => { adapter['filter:='] = jest.fn(() => 'test') - adapter.filter(builder, filter) - + expect(adapter.filter(builder, filter)).toBe('test') expect(adapter['filter:=']).toHaveBeenCalledWith(builder, filter) - expect(adapter['filter:=']).toHaveReturnedWith('test') FILTER_OPERATORS.mockRestore() }) @@ -86,10 +84,8 @@ describe('filter', () => { adapter['filter:*'] = jest.fn(() => 'test') - adapter.filter(builder, filter) - + expect(adapter.filter(builder, filter)).toBe('test') expect(adapter['filter:*']).toHaveBeenCalledWith(builder, filter) - expect(adapter['filter:*']).toHaveReturnedWith('test') FILTER_OPERATORS.mockRestore() }) diff --git a/src/parsers/page.js b/src/parsers/page.js index 0d389cd..8a1dd34 100644 --- a/src/parsers/page.js +++ b/src/parsers/page.js @@ -58,10 +58,7 @@ class PageParser extends BaseParser { if (!this.query) { page = this.defaults - } else if ( - typeof this.query === 'number' || - typeof this.query === 'string' - ) { + } else if (typeof this.query === 'number') { page = this.parseNumber() } else { page = this.parseObject() diff --git a/src/parsers/page.test.js b/src/parsers/page.test.js index fb21569..43b0a76 100644 --- a/src/parsers/page.test.js +++ b/src/parsers/page.test.js @@ -114,7 +114,7 @@ describe('flatten', () => { }) describe('parse', () => { - test('`page=number` with a string number', () => { + test('`page=number` coerces a string number', () => { const parser = new PageParser('page', '2', new Schema()) const parsed = parser.parse() diff --git a/src/validators/adapter.test.js b/src/validators/adapter.test.js index 393fce5..54e1464 100644 --- a/src/validators/adapter.test.js +++ b/src/validators/adapter.test.js @@ -85,13 +85,13 @@ describe('validateFilters', () => { expect(validator.validateFilters(parser.parse())).toBeInstanceOf(Map) }) - test('returns the parsed filters if all filters are valid', () => { + test('returns the parsed filters with the validated values', () => { const parser = new FilterParser( 'filter', { test: { - '=': 123, - '!=': 456, + '=': '123', + '!=': '456', }, }, new Schema().filter('test', '=').filter('test', '!='), @@ -101,7 +101,10 @@ describe('validateFilters', () => { 'filter:!=': schema.number(), })) - expect(validator.validateFilters(parser.parse())).toBeInstanceOf(Map) + const filters = validator.validateFilters(parser.parse()) + + expect(filters.get('filter:test[=]').value).toBe(123) + expect(filters.get('filter:test[!=]').value).toBe(456) }) test('throws `ValidationError` if a filter is invalid', () => { @@ -129,17 +132,20 @@ describe('validateSorts', () => { expect(validator.validateSorts(parser.parse())).toBeInstanceOf(Map) }) - test('returns the parsed sorts if all sorts are valid', () => { + test('returns the parsed sorts with the validated orders', () => { const parser = new SortParser( 'sort', ['test1', 'test2'], new Schema().sort('test1').sort('test2'), ) const validator = new AdapterValidator((schema) => ({ - sort: schema.string().valid('asc'), + sort: schema.string().uppercase(), })) - expect(validator.validateSorts(parser.parse())).toBeInstanceOf(Map) + const sorts = validator.validateSorts(parser.parse()) + + expect(sorts.get('sort:test1').order).toBe('ASC') + expect(sorts.get('sort:test2').order).toBe('ASC') }) test('throws `ValidationError` if a sort is invalid', () => { @@ -163,14 +169,17 @@ describe('validatePage', () => { expect(validator.validatePage(parser.parse())).toBeInstanceOf(Map) }) - test('returns the parsed page if page is valid', () => { + test('returns the parsed page with the validated values', () => { const parser = new PageParser('page', '2', new Schema()) const validator = new AdapterValidator((schema) => ({ 'page:size': schema.number().valid(20), 'page:number': schema.number().valid(2), })) - expect(validator.validatePage(parser.parse())).toBeInstanceOf(Map) + const page = validator.validatePage(parser.parse()) + + expect(page.get('page:size').value).toBe(20) + expect(page.get('page:number').value).toBe(2) }) test('throws `ValidationError` if page is invalid', () => { diff --git a/src/validators/querier/joi.test.js b/src/validators/querier/joi.test.js index b206308..522618c 100644 --- a/src/validators/querier/joi.test.js +++ b/src/validators/querier/joi.test.js @@ -55,7 +55,7 @@ describe('validateValue', () => { 'filter:test[=]': schema.number(), })) - expect(() => validator.schema.extract('filter:text[!=]')).toThrow() + expect(() => validator.schema.extract('filter:test[!=]')).toThrow() expect(validator.validateValue('filter:test[!=]', 123)).toBe(123) }) @@ -91,13 +91,13 @@ describe('validateFilters', () => { expect(validator.validateFilters(parser.parse())).toBeInstanceOf(Map) }) - test('returns the parsed filters if all filters are valid', () => { + test('returns the parsed filters with the validated values', () => { const parser = new FilterParser( 'filter', { test: { - '=': 123, - '!=': 456, + '=': '123', + '!=': '456', }, }, new Schema().filter('test', '=').filter('test', '!='), @@ -107,7 +107,10 @@ describe('validateFilters', () => { 'filter:test[!=]': schema.number(), })) - expect(validator.validateFilters(parser.parse())).toBeInstanceOf(Map) + const filters = validator.validateFilters(parser.parse()) + + expect(filters.get('filter:test[=]').value).toBe(123) + expect(filters.get('filter:test[!=]').value).toBe(456) }) test('throws `ValidationError` if a filter is invalid', () => { @@ -135,17 +138,20 @@ describe('validateSorts', () => { expect(validator.validateSorts(parser.parse())).toBeInstanceOf(Map) }) - test('returns the parsed sorts if all sorts are valid', () => { + test('returns the parsed sorts with the validated orders', () => { const parser = new SortParser( 'sort', ['test1', 'test2'], new Schema().sort('test1').sort('test2'), ) const validator = new JoiValidator((schema) => ({ - 'sort:test1': schema.string().valid('asc'), + 'sort:test1': schema.string().uppercase(), })) - expect(validator.validateSorts(parser.parse())).toBeInstanceOf(Map) + const sorts = validator.validateSorts(parser.parse()) + + expect(sorts.get('sort:test1').order).toBe('ASC') + expect(sorts.get('sort:test2').order).toBe('asc') }) test('throws `ValidationError` if a sort is invalid', () => { @@ -169,14 +175,17 @@ describe('validatePage', () => { expect(validator.validatePage(parser.parse())).toBeInstanceOf(Map) }) - test('returns the parsed page if page is valid', () => { + test('returns the parsed page with the validated values', () => { const parser = new PageParser('page', '2', new Schema()) const validator = new JoiValidator((schema) => ({ 'page:size': schema.number().valid(20), 'page:number': schema.number().valid(2), })) - expect(validator.validatePage(parser.parse())).toBeInstanceOf(Map) + const page = validator.validatePage(parser.parse()) + + expect(page.get('page:size').value).toBe(20) + expect(page.get('page:number').value).toBe(2) }) test('throws `ValidationError` if page is invalid', () => {