fix(tests): Adjust tests to use data-testid and document all selectors

Signed-off-by: Ferdinand Thiessen <[email protected]>

README.md

@@ -101,7 +101,19 @@ const paths = await filepicker.pick()
 </script>
 ```
 
-## Releasing a new version
+## Development
+### Testing
+For testing all components provide `data-testid` attributes as selectors, so the tests are independent from code or styling changes.
+
+### Test selectors
+`data-testid` | Intended purpose
+---|---
+`select-all-checkbox` | The select all checkbox of the file list
+`file-list-row`       | A row in the file list (`tr`), can be identified by `data-filename`
+`row-checkbox`        | Checkbox for selecting a row
+`row-name`            | Name of the row / file
+
+### Releasing a new version
 
 - Pull the latest changes from `master` or `stableX`;
 - Checkout a new branch with the tag name (e.g `v4.0.1`): `git checkout -b v<version>`;

l10n/messages.pot

@@ -102,7 +102,7 @@ msgstr ""
 msgid "Undo"
 msgstr ""
 
-#: lib/components/FilePicker/FileListRow.vue:26
+#: lib/components/FilePicker/FileListRow.vue:34
 msgid "Unset"
 msgstr ""
 

lib/components/FilePicker/FileList.spec.ts

@@ -120,11 +120,12 @@ describe('FilePicker FileList', () => {
 				path: '/',
 			},
 		})
-		expect(wrapper.find('th.row-checkbox').exists()).toBe(true)
+		const selectAll = wrapper.find('[data-testid="select-all-checkbox"]')
+		expect(selectAll.exists()).toBe(true)
 		// there is an aria label
-		expect(wrapper.find('[data-test="file-picker_select-all"]').attributes('aria-label')).toBeTruthy()
+		expect(selectAll.attributes('aria-label')).toBeTruthy()
 		// no checked
-		expect(wrapper.find('[data-test="file-picker_select-all"]').props('checked')).toBe(false)
+		expect(selectAll.props('checked')).toBe(false)
 	})
 
 	it('header checkbox is checked when all nodes are selected', async () => {
@@ -140,7 +141,7 @@ describe('FilePicker FileList', () => {
 			},
 		})
 
-		const selectAll = wrapper.find('[data-test="file-picker_select-all"]')
+		const selectAll = wrapper.find('[data-testid="select-all-checkbox"]')
 		expect(selectAll.props('checked')).toBe(true)
 	})
 
@@ -158,15 +159,15 @@ describe('FilePicker FileList', () => {
 				},
 			})
 
-			const rows = wrapper.findAll('.file-picker__row')
+			const rows = wrapper.findAll('[data-testid="file-list-row"]')
 			// all nodes are shown
 			expect(rows.length).toBe(nodes.length)
 			// folder are sorted first
-			expect(rows.at(0).attributes('data-file')).toBe('directory')
+			expect(rows.at(0).attributes('data-filename')).toBe('directory')
 			// other files are ascending
-			expect(rows.at(1).attributes('data-file')).toBe('a-file.txt')
-			expect(rows.at(2).attributes('data-file')).toBe('b-file.txt')
-			expect(rows.at(3).attributes('data-file')).toBe('favorite.txt')
+			expect(rows.at(1).attributes('data-filename')).toBe('a-file.txt')
+			expect(rows.at(2).attributes('data-filename')).toBe('b-file.txt')
+			expect(rows.at(3).attributes('data-filename')).toBe('favorite.txt')
 		})
 
 		it('can sort descending by name', async () => {
@@ -188,11 +189,11 @@ describe('FilePicker FileList', () => {
 			// all nodes are shown
 			expect(rows.length).toBe(nodes.length)
 			// folder are sorted first
-			expect(rows.at(0).attributes('data-file')).toBe('directory')
+			expect(rows.at(0).attributes('data-filename')).toBe('directory')
 			// other files are descending
-			expect(rows.at(1).attributes('data-file')).toBe('favorite.txt')
-			expect(rows.at(2).attributes('data-file')).toBe('b-file.txt')
-			expect(rows.at(3).attributes('data-file')).toBe('a-file.txt')
+			expect(rows.at(1).attributes('data-filename')).toBe('favorite.txt')
+			expect(rows.at(2).attributes('data-filename')).toBe('b-file.txt')
+			expect(rows.at(3).attributes('data-filename')).toBe('a-file.txt')
 		})
 	})
 })

lib/components/FilePicker/FileList.vue

@@ -10,7 +10,7 @@
 						<NcCheckboxRadioSwitch v-if="multiselect"
 							:aria-label="t('Select all entries')"
 							:checked="allSelected"
-							data-test="file-picker_select-all"
+							data-testid="select-all-checkbox"
 							@update:checked="onSelectAll" />
 					</th>
 					<th :aria-sort="sortByName" class="row-name">

lib/components/FilePicker/FileListRow.spec.ts

@@ -52,8 +52,9 @@ describe('FilePicker: FileListRow', () => {
 
 		const wrapper = mount(FileListRow, {
 			propsData: {
-				allowPickDirectory: false,
+				allowPickDirectory: true,
 				selected: false,
+				showCheckbox: true,
 				canPick: true,
 				node,
 			},
@@ -63,22 +64,73 @@ describe('FilePicker: FileListRow', () => {
 		expect(consoleWarn).not.toBeCalled()
 		expect(consoleError).not.toBeCalled()
 		// mounted
-		expect(wrapper.isEmpty()).toBe(false)
+		expect(wrapper.exists()).toBe(true)
 		expect(wrapper.element.tagName.toLowerCase()).toBe('tr')
-		expect(wrapper.element.classList.contains('file-picker__row')).toBe(true)
+		expect(wrapper.find('[data-testid="file-list-row"]').isEmpty()).toBe(false)
+	})
+
+	it('shows checkbox based on `showCheckbox` property', async () => {
+		const wrapper = mount(FileListRow, {
+			propsData: {
+				allowPickDirectory: true,
+				selected: false,
+				showCheckbox: true,
+				canPick: true,
+				node,
+			},
+		})
+
+		expect(wrapper.find('[data-testid="row-checkbox"]').exists()).toBe(true)
+		await wrapper.setProps({ showCheckbox: false })
+		expect(wrapper.find('[data-testid="row-checkbox"]').exists()).toBe(false)
+	})
+
+	it('Click checkbox triggers select', async () => {
+		const wrapper = mount(FileListRow, {
+			propsData: {
+				allowPickDirectory: false,
+				selected: false,
+				showCheckbox: true,
+				canPick: true,
+				node,
+			},
+		})
+
+		await wrapper.find('[data-testid="row-checkbox"]').trigger('click')
+
+		// one event with payload `true` is expected
+		expect(wrapper.emitted('update:selected')).toEqual([[true]])
+	})
+
+	it('Click element triggers select', async () => {
+		const wrapper = mount(FileListRow, {
+			propsData: {
+				allowPickDirectory: false,
+				selected: false,
+				showCheckbox: true,
+				canPick: true,
+				node,
+			},
+		})
+
+		await wrapper.find('[data-testid="row-name"]').trigger('click')
+
+		// one event with payload `true` is expected
+		expect(wrapper.emitted('update:selected')).toEqual([[true]])
 	})
 
-	it('Click triggers select', async () => {
+	it('Click element without checkbox triggers select', async () => {
 		const wrapper = mount(FileListRow, {
 			propsData: {
 				allowPickDirectory: false,
 				selected: false,
+				showCheckbox: false,
 				canPick: true,
 				node,
 			},
 		})
 
-		await wrapper.find('.row-checkbox *').trigger('click')
+		await wrapper.find('[data-testid="row-name"]').trigger('click')
 
 		// one event with payload `true` is expected
 		expect(wrapper.emitted('update:selected')).toEqual([[true]])
@@ -89,6 +141,7 @@ describe('FilePicker: FileListRow', () => {
 			propsData: {
 				allowPickDirectory: false,
 				selected: false,
+				showCheckbox: false,
 				canPick: true,
 				node,
 			},

lib/components/FilePicker/FileListRow.vue

@@ -4,21 +4,23 @@
 		:class="['file-picker__row', {
 			'file-picker__row--selected': selected && !showCheckbox
 		}]"
-		:data-file="node.basename"
+		:data-filename="node.basename"
+		data-testid="file-list-row"
 		@click="handleClick"
-		v-on="{
-			// same as tabindex -> if we hide the checkbox or this is a directory we need keyboard access to enter the directory or select the node
-			keydown: (showCheckbox && !isDirectory) ? null : handleKeyDown
-		}">
+		v-on="
+			/* same as tabindex -> if we hide the checkbox or this is a directory we need keyboard access to enter the directory or select the node */
+			(!showCheckbox || isDirectory) ? { keydown: handleKeyDown } : {}
+		">
 		<td v-if="showCheckbox" class="row-checkbox">
 			<NcCheckboxRadioSwitch :disabled="!isPickable"
 				:checked="selected"
 				:aria-label="t('Select the row for {nodename}', { nodename: displayName })"
+				data-testid="row-checkbox"
 				@click.stop="/* just stop the click event */"
 				@update:checked="toggleSelected" />
 		</td>
 		<td class="row-name">
-			<div class="file-picker__name-container">
+			<div class="file-picker__name-container" data-testid="row-name">
 				<div class="file-picker__file-icon" :style="{ backgroundImage }" />
 				<div class="file-picker__file-name" :title="displayName" v-text="displayName" />
 				<div class="file-picker__file-extension" v-text="fileExtension" />