Fix OS switching not working on tree view UI

This commit resolves a rendering bug in the tree view component.
Previously, updating the tree collection prior to node updates led to
rendering errors due to the presence of non-existent nodes in the new
collection.

Changes:

- Implement manual control over the rendering process in tree view. This
  includes clearing the rendering queue and currently rendered nodes
  before updates, aligning the rendering process with the updated
  collection.
- Add Cypress E2E tests to test switching between all operating systems
  and script views, ensuring no uncaught errors and preventing
  regression.
- Replace hardcoded operating system lists in the download URL list view
  with a unified `getSupportedOsList()` method from the application,
  reducing duplication and simplifying future updates.
- Rename `initial-nodes` to `nodes` in `TreeView.vue` to reflect their
  mutable nature.
- Centralize the function for getting operating system names into
  `OperatingSystemNames.ts`, improving reusability in E2E tests.
This commit is contained in:
undergroundwires
2023-12-14 09:51:42 +01:00
parent fe3de498c8
commit 3457fe18cf
13 changed files with 285 additions and 76 deletions

View File

@@ -2,8 +2,8 @@ import { expectExists } from '@tests/shared/Assertions/ExpectExists';
import { openCard } from './support/interactions/card';
describe('script selection highlighting', () => {
// Regression test for a bug where selecting multiple scripts only highlighted the last one.
it('highlights more when multiple scripts are selected', () => {
// Regression test for a bug where selecting multiple scripts only highlighted the last one.
cy.visit('/');
selectLastScript();
getCurrentHighlightRange((lastScriptHighlightRange) => {

View File

@@ -0,0 +1,72 @@
import { ViewType } from '@/presentation/components/Scripts/Menu/View/ViewType';
import { getEnumValues } from '@/application/Common/Enum';
import { OperatingSystem } from '@/domain/OperatingSystem';
import { getOperatingSystemDisplayName } from '@/presentation/components/Shared/OperatingSystemNames';
describe('operating system selector', () => {
// Regression test for a bug where switching between operating systems caused uncaught exceptions.
describe('allows user to switch between supported operating systems', () => {
getEnumValues(ViewType).forEach((viewType) => {
it(`switches to ${ViewType[viewType]} view successfully`, () => {
// arrange
cy.visit('/');
selectViewType(viewType);
getSupportedOperatingSystemsList().forEach((operatingSystem) => {
// act
selectOperatingSystem(operatingSystem);
// assert
assertExpectedActions();
});
});
});
});
});
function getSupportedOperatingSystemsList() {
/*
Marked: refactor-with-aot-compilation
The operating systems list is hardcoded due to the challenge of loading
the application within Cypress, as its compilation is tightly coupled with Vite.
Ideally, this should dynamically fetch the list from the compiled application.
*/
return [
OperatingSystem.Windows,
OperatingSystem.Linux,
OperatingSystem.macOS,
];
}
function assertExpectedActions() {
/*
Marked: refactor-with-aot-compilation
Assertions are currently hardcoded due to the inability to load the application within
Cypress, as compilation is tightly coupled with Vite. Future refactoring should dynamically
assert the visibility of all actions (e.g., `actions.map((a) => cy.contains(a.title))`)
once the application's compilation process is decoupled from Vite.
*/
cy.contains('Privacy cleanup');
}
function selectOperatingSystem(operatingSystem: OperatingSystem) {
const operatingSystemLabel = getOperatingSystemDisplayName(operatingSystem);
if (!operatingSystemLabel) {
throw new Error(`Label does not exist for operating system: ${OperatingSystem[operatingSystem]}`);
}
cy.log(`Visiting operating system: ${operatingSystemLabel}`);
cy
.contains('span', operatingSystemLabel)
.click();
}
function selectViewType(viewType: ViewType): void {
const viewTypeLabel = ViewTypeLabels[viewType];
cy.log(`Selecting view: ${ViewType[viewType]}`);
cy
.contains('span', viewTypeLabel)
.click();
}
const ViewTypeLabels: Record<ViewType, string> = {
[ViewType.Cards]: 'Cards',
[ViewType.Tree]: 'Tree',
} as const;

View File

@@ -13,6 +13,6 @@
"sourceMap": false,
},
"include": [
"**/*.ts"
"**/*.ts",
]
}

View File

@@ -0,0 +1,24 @@
import {
describe, it, expect,
} from 'vitest';
import { ApplicationFactory } from '@/application/ApplicationFactory';
import { getOperatingSystemDisplayName } from '@/presentation/components/Shared/OperatingSystemNames';
import { OperatingSystem } from '@/domain/OperatingSystem';
describe('OperatingSystemNames', () => {
describe('getOperatingSystemDisplayName', () => {
describe('retrieving display names for supported operating systems', async () => {
// arrange
const application = await ApplicationFactory.Current.getApp();
const supportedOperatingSystems = application.getSupportedOsList();
supportedOperatingSystems.forEach((supportedOperatingSystem) => {
it(`should return a non-empty name for ${OperatingSystem[supportedOperatingSystem]}`, () => {
// act
const displayName = getOperatingSystemDisplayName(supportedOperatingSystem);
// assert
expect(displayName).to.have.length.greaterThanOrEqual(1);
});
});
});
});
});

View File

@@ -9,23 +9,43 @@ import { provideDependencies } from '@/presentation/bootstrapping/DependencyProv
import { ApplicationContextStub } from '@tests/unit/shared/Stubs/ApplicationContextStub';
describe('TreeView', () => {
it('should render all provided root nodes correctly', async () => {
it('renders all provided root nodes correctly', async () => {
// arrange
const nodes = createSampleNodes();
const wrapper = createTreeViewWrapper(nodes);
// act
await waitForStableDom(wrapper.element);
// assert
const expectedTotalRootNodes = nodes.length;
expect(wrapper.findAll('.node').length).to.equal(expectedTotalRootNodes, wrapper.html());
const rootNodeTexts = nodes.map((node) => node.data.label);
const rootNodeTexts = nodes.map((node) => (node.data as TreeInputMetadata).label);
rootNodeTexts.forEach((label) => {
expect(wrapper.text()).to.include(label);
});
});
// Regression test for a bug where updating the nodes prop caused uncaught exceptions.
it('updates nodes correctly when props change', async () => {
// arrange
const firstNodeLabel = 'Node 1';
const secondNodeLabel = 'Node 2';
const initialNodes: TreeInputNodeDataWithMetadata[] = [{ id: 'node1', data: { label: firstNodeLabel } }];
const updatedNodes: TreeInputNodeDataWithMetadata[] = [{ id: 'node2', data: { label: secondNodeLabel } }];
const wrapper = createTreeViewWrapper(initialNodes);
// act
await wrapper.setProps({ nodes: updatedNodes });
await waitForStableDom(wrapper.element);
// assert
expect(wrapper.text()).toContain(secondNodeLabel);
expect(wrapper.text()).not.toContain(firstNodeLabel);
});
});
function createTreeViewWrapper(initialNodeData: readonly TreeInputNodeData[]) {
function createTreeViewWrapper(initialNodeData: readonly TreeInputNodeDataWithMetadata[]) {
return mount(defineComponent({
components: {
TreeView,
@@ -33,26 +53,34 @@ function createTreeViewWrapper(initialNodeData: readonly TreeInputNodeData[]) {
setup() {
provideDependencies(new ApplicationContextStub());
const initialNodes = shallowRef(initialNodeData);
const nodes = shallowRef(initialNodeData);
const selectedLeafNodeIds = shallowRef<readonly string[]>([]);
return {
initialNodes,
nodes,
selectedLeafNodeIds,
};
},
template: `
<TreeView
:initialNodes="initialNodes"
:selectedLeafNodeIds="selectedLeafNodeIds"
>
<template v-slot:node-content="{ nodeMetadata }">
{{ nodeMetadata.label }}
</template>
</TreeView>`,
<TreeView
:nodes="nodes"
:selectedLeafNodeIds="selectedLeafNodeIds"
>
<template v-slot:node-content="{ nodeMetadata }">
{{ nodeMetadata.label }}
</template>
</TreeView>
`,
}));
}
function createSampleNodes() {
interface TreeInputMetadata {
readonly label: string;
}
type TreeInputNodeDataWithMetadata = TreeInputNodeData & { readonly data?: TreeInputMetadata };
function createSampleNodes(): TreeInputNodeDataWithMetadata[] {
return [
{
id: 'root1',
@@ -93,7 +121,7 @@ function createSampleNodes() {
function waitForStableDom(rootElement, timeout = 3000, interval = 200): Promise<void> {
return new Promise((resolve, reject) => {
let lastTimeoutId;
let lastTimeoutId: ReturnType<typeof setTimeout>;
const observer = new MutationObserver(() => {
if (lastTimeoutId) {
clearTimeout(lastTimeoutId);

View File

@@ -1,5 +1,7 @@
import { describe, it, expect } from 'vitest';
import { type Ref, shallowRef } from 'vue';
import {
type Ref, shallowRef, watch, nextTick,
} from 'vue';
import { useGradualNodeRendering } from '@/presentation/components/Scripts/View/Tree/TreeView/Rendering/UseGradualNodeRendering';
import { TreeRoot } from '@/presentation/components/Scripts/View/Tree/TreeView/TreeRoot/TreeRoot';
import { TreeRootStub } from '@tests/unit/shared/Stubs/TreeRootStub';
@@ -70,9 +72,9 @@ describe('useGradualNodeRendering', () => {
.withOldState(new TreeNodeStateDescriptorStub().withVisibility(oldVisibilityState))
.withNewState(new TreeNodeStateDescriptorStub().withVisibility(newVisibilityState));
// act
const strategy = builder.call();
const { renderingStrategy } = builder.call();
aggregatorStub.notifyChange(change);
const actualRenderStatus = strategy.shouldRender(node);
const actualRenderStatus = renderingStrategy.shouldRender(node);
// assert
expect(actualRenderStatus).to.equal(expectedRenderStatus);
});
@@ -186,11 +188,11 @@ describe('useGradualNodeRendering', () => {
.withSubsequentBatchSize(subsequentBatchSize)
.withDelayScheduler(delaySchedulerStub);
// act
const strategy = builder.call();
const { renderingStrategy } = builder.call();
Array.from({ length: schedulerTicks }).forEach(
() => delaySchedulerStub.runNextScheduled(),
);
const actualRenderStatuses = nodes.map((node) => strategy.shouldRender(node));
const actualRenderStatuses = nodes.map((node) => renderingStrategy.shouldRender(node));
// expect
expect(actualRenderStatuses).to.deep.equal(expectedRenderStatuses);
});
@@ -218,9 +220,9 @@ describe('useGradualNodeRendering', () => {
const actualOrder = new Set<ReadOnlyTreeNode>();
// act
ordererStub.orderNodes = () => expectedNodes[0];
const strategy = builder.call();
const { renderingStrategy } = builder.call();
const updateOrder = () => allNodes
.filter((node) => strategy.shouldRender(node))
.filter((node) => renderingStrategy.shouldRender(node))
.forEach((node) => actualOrder.add(node));
updateOrder();
for (let i = 1; i < expectedNodes.length; i++) {
@@ -247,6 +249,48 @@ describe('useGradualNodeRendering', () => {
// assert
expect(delaySchedulerStub.nextCallback).toBeUndefined();
});
describe('clearRenderingStates', () => {
it('clears all nodes from rendering states', () => {
// arrange
const nodesStub = [new TreeNodeStub(), new TreeNodeStub()];
const rendering = new UseGradualNodeRenderingBuilder()
.withCurrentTreeNodes(new UseCurrentTreeNodesStub()
.withQueryableNodes(new QueryableNodesStub().withFlattenedNodes(nodesStub)))
.call();
// act
rendering.clearRenderingStates();
// assert
const isAnyRendered = nodesStub
.map((node) => rendering.renderingStrategy.shouldRender(node))
.some(Boolean);
expect(isAnyRendered).to.equal(false);
});
});
describe('notifyRenderingUpdates', () => {
it('triggers Vue reactivity update', async () => {
// arrange
const nodes = createNodesWithVisibility(true, 1);
const rendering = new UseGradualNodeRenderingBuilder()
.withCurrentTreeNodes(new UseCurrentTreeNodesStub()
.withQueryableNodes(new QueryableNodesStub().withFlattenedNodes(nodes)))
.withInitialBatchSize(nodes.length)
.call();
let isVueReactivityTriggered = false;
watch(() => rendering.renderingStrategy.shouldRender(nodes[0]), () => {
isVueReactivityTriggered = true;
});
// act
rendering.clearRenderingStates();
rendering.notifyRenderingUpdates();
await nextTick();
// assert
expect(isVueReactivityTriggered).toEqual(true);
});
});
});
function createNodesWithVisibility(