fix: remove unnecessary CI checks in tests, fix issue with history (#32)

Author: mateuszJS

integration-tests/index.ts

@@ -26,10 +26,11 @@ async function test() {
       window.assetsSnapshot = assets
       // we had to implement this whole history logic because there is no way
       // to call creator.resetCanvas(newAssets) from test code file
-      if (currentHistoryIndex === assetsUpdatesHistory.length - 1) {
-        assetsUpdatesHistory.push(assets)
-        currentHistoryIndex = assetsUpdatesHistory.length - 1
+      if (currentHistoryIndex < assetsUpdatesHistory.length - 1) {
+        assetsUpdatesHistory.splice(currentHistoryIndex + 1)
       }
+      assetsUpdatesHistory.push(assets)
+      currentHistoryIndex = assetsUpdatesHistory.length - 1
       console.log(assets)
     },
     (assetId) => {
@@ -48,8 +49,10 @@ async function test() {
     addImageInput.value = '' // reset input value to allow re-uploading the same file
   })
 
-  const startProjectInput = document.querySelector<HTMLInputElement>('#start-project-from-images')!
-  startProjectInput.addEventListener('change', (event) => {
+  const startProjectInputFromImages = document.querySelector<HTMLInputElement>(
+    '#start-project-from-images'
+  )!
+  startProjectInputFromImages.addEventListener('change', (event) => {
     const { files } = event.target as HTMLInputElement
     if (!files) return
     creator.resetAssets(
@@ -58,7 +61,28 @@ async function test() {
       })),
       true
     )
-    startProjectInput.value = '' // reset input value to allow re-uploading the same file
+    startProjectInputFromImages.value = '' // reset input value to allow re-uploading the same file
+  })
+
+  const startProjectInputFroMAssets = document.querySelector<HTMLInputElement>(
+    '#start-project-from-assets'
+  )!
+  startProjectInputFroMAssets.addEventListener('change', (event) => {
+    const { files } = event.target as HTMLInputElement
+    if (!files) return
+
+    const PROJECT_SAMPLE = Array.from(files).map((file) => ({
+      url: URL.createObjectURL(file),
+      points: [
+        { x: 100, y: 100, u: 0, v: 0 },
+        { x: 200, y: 100, u: 1, v: 0 },
+        { x: 200, y: 200, u: 1, v: 1 },
+        { x: 100, y: 200, u: 0, v: 1 },
+      ],
+    }))
+
+    creator.resetAssets(PROJECT_SAMPLE, true)
+    startProjectInputFroMAssets.value = '' // reset input value to allow re-uploading the same file
   })
 
   removeAssetBtn.addEventListener('click', () => {

integration-tests/template.html

@@ -32,6 +32,7 @@
   <aside>
     <label>Add image<input type="file" id="add-image" /></label>
     <label>Start project from images<input type="file" multiple id="start-project-from-images" /></label>
+    <label>Start project from project assets<input type="file" multiple id="start-project-from-assets" /></label>
     <p>Selected asset: <span id="selected-asset-id">0</span></p>
     <p>Is processing events: <span id="is-processing-events">false</span></p>
     <button id="remove-btn">Remove Asset</button>

integration-tests/tests/asset-basic-transform.spec.ts

@@ -4,11 +4,6 @@ import { test, expect } from '@playwright/test'
 import init from '../init'
 
 test('asset performs basic transformations', async ({ page }, testinfo) => {
-  if (process.env.CI) {
-    test.skip()
-    return
-  }
-
   testinfo.snapshotSuffix = '' // by default is `process.platform`
 
   const utils = await init(page)

integration-tests/tests/asset-removal.spec.ts

@@ -4,11 +4,6 @@ import { test, expect } from '@playwright/test'
 import init from '../init'
 
 test('asset removal', async ({ page }, testinfo) => {
-  if (process.env.CI) {
-    test.skip()
-    return
-  }
-
   testinfo.snapshotSuffix = '' // by default is `process.platform`
 
   const utils = await init(page)

integration-tests/tests/assets-snapshot.spec.ts

@@ -39,12 +39,7 @@ const STATE_AFTER_UPLOAD = [
 ]
 
 test('asset selection', async ({ page }, testinfo) => {
-  if (process.env.CI) {
-    test.skip()
-    return
-  }
   testinfo.snapshotSuffix = '' // by default is `process.platform`
-  const canvas = page.locator('canvas')
   const assetIdEl = page.locator('#selected-asset-id')
 
   const utils = await init(page)

integration-tests/tests/history.spec.ts

@@ -3,11 +3,7 @@
 import { test, expect } from '@playwright/test'
 import init, { TransformHandle } from '../init'
 
-test('asset selection', async ({ page }, testinfo) => {
-  if (process.env.CI) {
-    test.skip()
-    return
-  }
+test('history', async ({ page }, testinfo) => {
   testinfo.snapshotSuffix = '' // by default is `process.platform`
   const undoBtn = page.locator('#undo-btn')
   const redoBtn = page.locator('#redo-btn')
@@ -43,3 +39,29 @@ test('asset selection', async ({ page }, testinfo) => {
   await redoBtn.click() // redo second asset upload
   expect(await utils.getAssetsState()).toStrictEqual(stateSecondAssetTransform)
 })
+
+test('history - the next update after reset_assets should be different than input of reset_assets', async ({
+  page,
+}, testinfo) => {
+  // There was an issue where we go back in history and last_snapshot was not updated(reset_asset wasn't updating it)
+  // and with next mouse event check_assets_update function was called with same data as reset_assets got
+  // while should not be called at all if nothing has changed
+
+  // this test case looks for this behaviour by checking if rolling back history was interrupted with just a mouse event(no real change in assets)
+
+  testinfo.snapshotSuffix = ''
+  const undoBtn = page.locator('#undo-btn')
+  const redoBtn = page.locator('#redo-btn')
+
+  const utils = await init(page)
+  const firstAsset = await utils.uploadAsset()
+
+  await utils.selectAsset(firstAsset)
+  await utils.resizeAsset(firstAsset, 200, 200, TransformHandle.BOTTOM_RIGHT)
+  const afterResizeState = await utils.getAssetsState()
+  await undoBtn.click()
+  await page.mouse.move(300, 300) // move pointer on canvas
+  await page.mouse.move(0, 0) // so now can be move out and trigger mouse leave event
+  await redoBtn.click()
+  expect(await utils.getAssetsState()).toStrictEqual(afterResizeState)
+})

integration-tests/tests/initial-assets.spec.ts

@@ -5,22 +5,19 @@ import init from '../init'
 import { fileURLToPath } from 'url'
 import path from 'path'
 
+const __dirname = path.dirname(fileURLToPath(import.meta.url))
+
 test('initial assets', async ({ page }, testinfo) => {
-  if (process.env.CI) {
-    test.skip()
-    return
-  }
   testinfo.snapshotSuffix = '' // by default is `process.platform`
 
   const utils = await init(page)
 
-  const __dirname = path.dirname(fileURLToPath(import.meta.url))
   const testImagePaths = [
     path.join(__dirname, '../image-sample.png'),
     path.join(__dirname, '../another-image-sample.jpg'),
   ]
 
-  const fileInput = (await page.$('#start-project-from-images'))!
+  const fileInput = (await page.$('#start-project-from-assets'))!
   await fileInput.setInputFiles(testImagePaths)
   const assets = await utils.getAssetsState()
   expect(assets.length).toBe(2)

integration-tests/tests/initial-images.spec.ts

@@ -0,0 +1,23 @@
+// npm run test-e2e -- initial-images.spec.ts --debug
+
+import { test, expect } from '@playwright/test'
+import init from '../init'
+import { fileURLToPath } from 'url'
+import path from 'path'
+
+test('initial images', async ({ page }, testinfo) => {
+  testinfo.snapshotSuffix = '' // by default is `process.platform`
+
+  const utils = await init(page)
+
+  const __dirname = path.dirname(fileURLToPath(import.meta.url))
+  const testImagePaths = [
+    path.join(__dirname, '../image-sample.png'),
+    path.join(__dirname, '../another-image-sample.jpg'),
+  ]
+
+  const fileInput = (await page.$('#start-project-from-images'))!
+  await fileInput.setInputFiles(testImagePaths)
+  const assets = await utils.getAssetsState()
+  expect(assets.length).toBe(2)
+})

integration-tests/tests/ui-hover.spec.ts

@@ -4,11 +4,6 @@ import { test, expect } from '@playwright/test'
 import init from '../init'
 
 test('ui elements get correct highlight on hover', async ({ page }, testinfo) => {
-  if (process.env.CI) {
-    test.skip()
-    return
-  }
-
   testinfo.snapshotSuffix = '' // by default is `process.platform`
 
   const utils = await init(page)

src/logic/index.zig

@@ -81,14 +81,14 @@ fn generate_id() u32 {
 pub fn add_asset(id_or_zero: u32, points: [4]Types.PointUV, texture_id: u32) void {
     const id = if (id_or_zero == 0) generate_id() else id_or_zero;
     state.assets.put(id, Texture.new(id, points, texture_id)) catch unreachable;
-    notify_about_assets_update();
+    check_assets_update(true);
 }
 
 pub fn remove_asset() void {
     _ = state.assets.remove(state.active_asset_id);
     state.active_asset_id = 0;
     on_asset_select_cb(state.active_asset_id);
-    notify_about_assets_update();
+    check_assets_update(true);
 }
 
 pub fn on_update_pick(id: u32) void {
@@ -111,7 +111,7 @@ pub fn on_pointer_down(x: f32, y: f32) void {
 
 // const std.heap.page_allocator.alloc(AssetZig, state.assets.count())
 var last_assets_update: []const AssetZig = &.{};
-fn notify_about_assets_update() void {
+fn check_assets_update(should_notify: bool) void {
     const cb = on_asset_update_cb orelse return;
 
     var new_assets_update = std.heap.page_allocator.alloc(AssetZig, state.assets.count()) catch unreachable;
@@ -140,10 +140,12 @@ fn notify_about_assets_update() void {
     std.heap.page_allocator.free(last_assets_update);
     last_assets_update = new_assets_update;
 
-    if (new_assets_update.len > 0) {
-        cb(new_assets_update); // would throw error if results.len == 0
-    } else {
-        cb(&.{});
+    if (should_notify) {
+        if (new_assets_update.len > 0) {
+            cb(new_assets_update); // would throw error if results.len == 0
+        } else {
+            cb(&.{});
+        }
     }
 }
 
@@ -153,7 +155,7 @@ pub fn on_pointer_up() void {
         on_asset_select_cb(state.active_asset_id);
     } else {
         state.ongoing_action = .none;
-        notify_about_assets_update();
+        check_assets_update(true);
     }
 }
 
@@ -192,7 +194,7 @@ pub fn on_pointer_move(x: f32, y: f32) void {
 pub fn on_pointer_leave() void {
     state.ongoing_action = .none;
     state.hovered_asset_id = 0;
-    notify_about_assets_update();
+    check_assets_update(true);
 }
 
 fn get_border() struct { []f32, []f32 } { // { triangle vertex, msdf vertex }
@@ -332,9 +334,7 @@ pub fn reset_assets(new_assets: []const AssetZig, with_snapshot: bool) void {
 
     on_asset_update_cb = real_callback_pointer;
 
-    if (with_snapshot) {
-        notify_about_assets_update();
-    }
+    check_assets_update(with_snapshot);
 }
 
 pub fn destroy_state() void {