Skip to content

Commit

Permalink
Fix off by one error in small files
Browse files Browse the repository at this point in the history
Currently there's an off-by-one error when displaying a small file with
fewer rows than the number used in previews (10).  This fixes the
off-by-one and introduces a test to ensure we don't re-introduce it.
  • Loading branch information
rossjones committed Sep 30, 2024
1 parent 1efb6fb commit d64f388
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 1 deletion.
2 changes: 2 additions & 0 deletions fixtures/small-file.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
A,B
1,2
2 changes: 1 addition & 1 deletion lib/importer/sheets.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ exports.GetRows = (session, start=0, count=10) => {
const rowRange = {
sheet: session.sheet,
start: { row: start, column: 0},
end: { row: Math.min(start + count - 1, sheetDimensions.rows-1), column: sheetDimensions.columns-1}
end: { row: Math.min(start + count, sheetDimensions.rows), column: sheetDimensions.columns-1}
}

const wantedRows = rowRange.end.row - start;
Expand Down
96 changes: 96 additions & 0 deletions prototypes/basic/tests/small-file.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import { test, expect } from '@playwright/test';

import * as fixtures from "./helpers/fixtures";

import { UploadPage } from './helpers/upload_page';
import { SheetSelectorPage } from './helpers/sheet_selector_page';
import { HeaderSelectorPage } from './helpers/header_selector_page';
import { FooterSelectorPage } from './helpers/footer_selector_page'
import { MappingPage } from './helpers/mapping_page'

const path = require('node:path');

// Take a screenshot to help with debugging
const screenshot = async(page, name) => {
await page.screenshot({ path: name, fullPage: true });
}

test('tiny files', async ({ page }) => {
await page.goto('/');
await page.getByText('Start now').click();

// ---------------------------------------------------------------------------
// Upload a file
await expect(page).toHaveURL(/.upload/)

const uploadPage = new UploadPage(page)
const browseButton = await uploadPage.browseButton()

await browseButton.click();
await browseButton.setInputFiles(fixtures.getFixture('small-file.csv'));
await uploadPage.submit();

// ---------------------------------------------------------------------------
// Select sheet
await expect(page).toHaveURL(/.select_sheet/)

const sheets = new SheetSelectorPage(page);

// There is only a single sheet, it should default be selected and the table
// visible
const radioButton = await sheets.getRadio('Sheet1')
await expect(radioButton).toBeChecked();
const vis = await sheets.getTableProperty(0, 'visibility')
await expect(vis).toBe('visible')

// // Check the data is present in the sheet previews
const previewRow = await sheets.getTableRow(0, 0)
const zerozero = await previewRow.locator("td").nth(0).textContent()
const zeroone = await previewRow.locator("td").nth(1).textContent()
await expect(zerozero).toBe("A")
await expect(zeroone).toBe("B")

await sheets.submit()

// ---------------------------------------------------------------------------
// Select some cells for a header row
// We will use the entire first row as our selection
await expect(page).toHaveURL(/.select_header_row/)

const headers = new HeaderSelectorPage(page)
await headers.select([0,0], [0,1])
await headers.submit()

// // ---------------------------------------------------------------------------
// // Do not choose a footer row, just click Skip
await expect(page).toHaveURL(/.select_footer_row/)

const footers = new FooterSelectorPage(page)
await footers.skip()

// // ---------------------------------------------------------------------------
// // Perform the mapping after checking the previews here are what we expect
await expect(page).toHaveURL(/.mapping/)

const mapping = new MappingPage(page)

const expectedColumns = ["A", "B"]
const expectedExamples = [
"1",
"2",
]

const colNames = await mapping.getColumnNames()
expect(colNames).toStrictEqual(expectedColumns)

Check failure on line 84 in prototypes/basic/tests/small-file.spec.js

View workflow job for this annotation

GitHub Actions / test

small-file.spec.js:18:5 › tiny files

1) small-file.spec.js:18:5 › tiny files ────────────────────────────────────────────────────────── Error: expect(received).toStrictEqual(expected) // deep equality - Expected - 1 + Received + 0 Array [ "A", - "B", ] 82 | 83 | const colNames = await mapping.getColumnNames() > 84 | expect(colNames).toStrictEqual(expectedColumns) | ^ 85 | 86 | const examples = await mapping.getExamples() 87 | expect(examples).toStrictEqual(expectedExamples) at /home/runner/work/data-import/data-import/prototypes/basic/tests/small-file.spec.js:84:22

const examples = await mapping.getExamples()
expect(examples).toStrictEqual(expectedExamples)

await mapping.setMapping('A', 'Code')
await mapping.setMapping('B', 'Name')
await mapping.submit()

// // ---------------------------------------------------------------------------
// // Should be on the success page
await expect(page).toHaveURL(/.success/)
});

0 comments on commit d64f388

Please sign in to comment.