Workflow updates (#908)

* enable Windows middleware tests

* Should fix windows CI for middleware

* git, do not modify test data files on checkout

* attempt to fix out-of-band change test under Windows

* fixes to enable Windows to pass the TS client tests

* adjustment to executeServer to handle the case where Windows is running a bash shell

* update yarn TS Client test to use powershell when run with Windows

* add files created by TS client tests to .gitignore

* refrain from running TS client lifecycle tests under Windows

* Windows test probe

* reorder TS client tests

* See if moving --exit location fixed windows test

* fix mocha bombing at the end of TS client testing in Windows

---------

Co-authored-by: Shane Dell <[email protected]>

.gitattributes

@@ -16,6 +16,7 @@
 # to native line endings on checkout.
 *.bat text
 *.c text
+*.cmake text
 *.cpp text
 *.gyp text
 *.h text
@@ -24,6 +25,8 @@
 *.js text
 *.json text
 *.md text
+*.proto text
+*.scala text
 *.sh text
 *.ts text
 *.yml text
@@ -32,3 +35,7 @@
 # Denote all files that are truly binary and should not be modified.
 *.dat binary
 *.png binary
+
+# Test data files should not be modified.
+core/src/tests/data/* binary
+packages/client/tests/specs/data/* binary

.github/workflows/build-middleware/action.yml

@@ -86,20 +86,11 @@ runs:
       run: sbt headerCheckAll
       working-directory: server/scala
 
-    - name: Package Scala API - Non windows 🎁
+    - name: Package Scala API 🎁
       shell: bash
-      run: sbt installM2 # runs test so specifically running sbt test not needed  # TODO: Make sure tests run on windows
-      if: inputs.runner-os != 'Windows'
+      run: sbt installM2 # runs test so specifically running sbt test not needed
       working-directory: server/scala
       # timeout-minutes: 30
-
-    - name: Package Scala Native - Windows 🎁
-      shell: bash
-      env:
-        GITHUB_TOKEN: ${{ inputs.github-token }}
-      if: inputs.runner-os == 'Windows'  # TODO: Remove, current workaround so we can download all OS jars from tests for release
-      run: sbt native/publishM2
-      working-directory: server/scala
 
     # - name: Archive M2 🔺
     #   uses: actions/upload-artifact@v4
@@ -112,22 +103,16 @@ runs:
     - name: Test Scala RPC server 📋
       shell: bash
       run: sbt serv/test
-      if: inputs.runner-os != 'Windows' # TODO: Make sure tests run on windows
       working-directory: server/scala
-      # timeout-minutes: 30
 
     - name: Yarn Install 🏗️
-      run: yarn
       shell: bash
+      run: yarn
 
     - name: Yarn Package - Server 📦
-      if: inputs.runner-os != 'Windows' # TODO: Make sure tests run on windows
-      run: yarn workspace @omega-edit/server package
       shell: bash
-      # timeout-minutes: 30
+      run: yarn workspace @omega-edit/server package
 
     - name: Yarn Test - Client 🧑‍💼
-      if: inputs.runner-os != 'Windows' # TODO: Make sure tests run on windows
+      shell: ${{ (inputs.runner-os == 'Windows') && 'pwsh' || 'bash' }}
       run: yarn workspace @omega-edit/client test
-      shell: bash
-      # timeout-minutes: 30

.gitignore

@@ -74,4 +74,5 @@ omega_edit_*.ts
 package
 src/*.js
 packages/client/src/client_version.ts
+packages/client/tests/specs/data/.OmegaEdit-orig.*
 omega-edit-grpc-server.*

packages/client/package.json

@@ -21,18 +21,23 @@
     "compile-src:default": "./compile-proto.sh",
     "compile-src:windows": "./compile-proto.bat",
     "docgen": "typedoc",
-    "prepackage": "yarn compile-src && yarn build",
-    "package": "yarn --cwd out cache clean && yarn --cwd out pack --install-if-needed -f omega-edit-node-client-v${npm_package_version}.tgz",
-    "install-client-local": "yarn add file://$INIT_CWD/omega-edit-node-client-v${npm_package_version}.tgz",
+    "prepackage": "yarn compile-src && yarn build && yarn --cwd out cache clean",
+    "package": "run-script-os",
+    "package:default": "yarn --cwd out pack --install-if-needed -f omega-edit-node-client-v${npm_package_version}.tgz",
+    "package:windows": "yarn --cwd out pack --install-if-needed -f omega-edit-node-client-v%npm_package_version%.tgz",
+    "install-client-local": "run-script-os",
+    "install-client-local:default": "yarn add file://$INIT_CWD/omega-edit-node-client-v${npm_package_version}.tgz",
+    "install-client-local:windows": "yarn add file:%INIT_CWD%/omega-edit-node-client-v%npm_package_version%.tgz",
     "pretest": "yarn package && yarn install-client-local",
-    "test:client": "mocha --exit --timeout 100000 --slow 50000 --require ts-node/register --require tests/fixtures.ts --exclude ./tests/specs/server.spec.ts ./tests/specs/*.spec.ts",
-    "test:lifecycle": "mocha --exit --timeout 50000 --slow 25000 --require ts-node/register ./tests/specs/server.spec.ts",
-    "test": "(yarn test:client && yarn test:lifecycle) || (yarn posttest && exit 1)",
+    "test:client": "mocha --timeout 100000 --slow 50000 --require ts-node/register --require tests/fixtures.ts --exclude ./tests/specs/server.spec.ts ./tests/specs/*.spec.ts --exit",
+    "test:lifecycle": "mocha --timeout 50000 --slow 35000 --require ts-node/register ./tests/specs/server.spec.ts --exit",
+    "test": "(yarn test:lifecycle && yarn test:client) || (yarn posttest && exit 1)",
     "posttest": "yarn remove @omega-edit/client",
     "lint": "prettier --check package.json webpack.config.js src tests && eslint .",
     "lint:fix": "prettier --write package.json webpack.config.js src tests && eslint --fix ."
   },
   "devDependencies": {
+    "clean-webpack-plugin": "^4.0.0",
     "grpc-tools": "^1.12.4",
     "grpc_tools_node_protoc_ts": "^5.3.3",
     "pino-webpack-plugin": "^2.0.0",

packages/client/src/viewport.ts

@@ -352,8 +352,10 @@ export async function unsubscribeViewport(
       .on('error', (err) => {
         // Call cancelled thrown when server is shutdown
         if (!err.message.includes('Call cancelled')) {
+          log.error('unsubscribeViewport critical error: ' + err.message)
           throw err
         }
+        log.info('unsubscribeViewport error: ' + err.message)
       })
   })
 }

packages/client/tests/specs/common.ts

@@ -107,7 +107,7 @@ export async function subscribeSession(
   const client = await getClient()
   client
     .subscribeToSessionEvents(subscriptionRequest)
-    .on('data', (sessionEvent) => {
+    .on('data', (sessionEvent): void => {
       session_callbacks.set(
         session_id,
         session_callbacks.has(session_id)
@@ -137,11 +137,16 @@ export async function subscribeSession(
         )
       }
     })
-    .on('error', (err) => {
+    .on('error', (err: Error): void => {
       // Call cancelled thrown when server is shutdown
-      if (!err.message.includes('Call cancelled')) {
+      if (
+        !err.message.includes('Call cancelled') &&
+        !err.message.includes('ECONNRESET')
+      ) {
+        log_error('subscribeSession critical error: ' + err.message)
         throw err
       }
+      log_info('subscribeSession error: ' + err.message)
     })
 
   return session_id
@@ -158,7 +163,7 @@ export async function subscribeViewport(
   const client = await getClient()
   client
     .subscribeToViewportEvents(subscriptionRequest)
-    .on('data', (viewportEvent) => {
+    .on('data', (viewportEvent): void => {
       viewport_callbacks.set(
         viewport_id,
         viewport_callbacks.has(viewport_id)
@@ -194,11 +199,16 @@ export async function subscribeViewport(
         )
       }
     })
-    .on('error', (err) => {
+    .on('error', (err: Error): void => {
       // Call cancelled thrown when server is shutdown
-      if (!err.message.includes('Call cancelled')) {
+      if (
+        !err.message.includes('Call cancelled') &&
+        !err.message.includes('ECONNRESET')
+      ) {
+        log_error('subscribeViewport critical error: ' + err.message)
         throw err
       }
+      log_info('subscribeViewport error: ' + err.message)
     })
 
   return viewport_id

packages/client/tests/specs/editing.spec.ts

@@ -130,6 +130,7 @@ describe('Editing', () => {
     it('Should overwrite some data', async () => {
       expect(await getComputedFileSize(session_id)).to.equal(0)
       const data: Uint8Array = Buffer.from('abcdefghijklmnopqrstuvwxyΩ') // Note: Ω is a 2-byte character
+      expect(data.length).equals(27)
       const stats = new EditStats()
       let change_id = await overwrite(session_id, 0, data, stats)
       expect(stats.overwrite_count).to.equal(1)
@@ -141,12 +142,14 @@ describe('Editing', () => {
       let file_size = await getComputedFileSize(session_id)
       expect(file_size).equals(data.length)
       let last_change = await getLastChange(session_id)
-      expect(last_change.getData_asU8()).deep.equals(data)
+      expect(last_change.getSessionId()).to.equal(session_id)
       expect(last_change.getOffset()).to.equal(0)
       expect(last_change.getKind()).to.equal(ChangeKind.CHANGE_OVERWRITE)
       expect(last_change.getSerial()).to.equal(1)
-      expect(last_change.getLength()).to.equal(27)
-      expect(last_change.getSessionId()).to.equal(session_id)
+      expect(last_change.getLength()).to.equal(data.length)
+      expect(last_change.getData().length).to.equal(data.length)
+      expect(last_change.getData_asU8().length).equals(data.length)
+      expect(last_change.getData_asU8()).deep.equals(data)
       let overwrite_change_id = await overwrite(
         session_id,
         13,

packages/client/tests/specs/server.spec.ts

@@ -123,8 +123,8 @@ describe('Server', () => {
   it(`on port ${serverTestPort} should stop immediately via API`, async () => {
     // stop the server immediately should stop the server immediately without waiting for sessions to end
     await stopServerImmediate()
-    // pause for up to 2 seconds to allow server some time to stop
-    for (let i = 0; i < 20; ++i) {
+    // pause for up to 3 seconds to allow server some time to stop
+    for (let i = 0; i < 30; ++i) {
       await delay(100) // 0.1 second
       if (!pidIsRunning(pid as number)) {
         break

packages/client/tests/specs/session.spec.ts

@@ -816,6 +816,10 @@ describe('Sessions', () => {
     expect(save_session_response.getFilePath()).to.equal(save1)
     fs.unlinkSync(save_session_response.getFilePath())
 
+    // pause for 1 second because that's the highest resolution supported by
+    // Windows file timestamps
+    await new Promise((resolve) => setTimeout(resolve, 1000))
+
     // touch the original file to simulate an out-of-band change
     touch(testFile)
 
@@ -860,9 +864,13 @@ describe('Sessions', () => {
   })
 
   it('Should be able to handle multiple simultaneous sessions', async () => {
+    expect(await getSessionCount()).to.equal(0)
     const session1 = await createSession()
-    const session_id1 = session1.getSessionId()
+    expect(await getSessionCount()).to.equal(1)
     const session2 = await createSession()
+    expect(await getSessionCount()).to.equal(2)
+
+    const session_id1 = session1.getSessionId()
     const session_id2 = session2.getSessionId()
     expect(session_id1).to.not.equal(session_id2)
     expect(session1.hasFileSize()).to.be.false

packages/client/tests/specs/viewport.spec.ts

@@ -176,6 +176,13 @@ describe('Viewports', () => {
     await checkCallbackCount(viewport_callbacks, viewport_1_id, 1)
     await checkCallbackCount(viewport_callbacks, viewport_2_id, 3)
     log_info(viewport_callbacks)
+    // Unsubscribe all viewporta
+    await unsubscribeViewport(viewport_1_id)
+    await unsubscribeViewport(viewport_2_id)
+    // Note the viewports are not destroyed, just unsubscribed
+    expect(await getViewportCount(session_id)).to.equal(1)
+    await destroyViewport(viewport_1_id)
+    expect(await getViewportCount(session_id)).to.equal(0)
   }).timeout(8000)
 
   it('Should handle floating viewports', async () => {
@@ -185,7 +192,7 @@ describe('Viewports', () => {
       Buffer.from('0123456789LABEL01234567890')
     )
     expect(change_id).to.equal(1)
-
+    expect(await getViewportCount(session_id)).to.equal(0)
     const viewport_response = await createViewport(
       'test_vpt_no_float',
       session_id,
@@ -194,6 +201,8 @@ describe('Viewports', () => {
     )
     const viewport_id = viewport_response.getViewportId()
     expect(await subscribeViewport(viewport_id)).to.equal(viewport_id)
+    expect(await getViewportCount(session_id)).to.equal(1)
+    expect(await viewportHasChanges(viewport_id)).to.be.false
     const viewport_floating_response = await createViewport(
       'test_vpt_label',
       session_id,
@@ -273,12 +282,22 @@ describe('Viewports', () => {
     expect(viewport_data.getLength()).to.equal(0)
     expect(viewport_data.getOffset()).to.equal(100)
     expect(viewport_data.getFollowingByteCount()).to.equal(-74)
+    await unsubscribeViewport(viewport_off_id)
+    await destroyViewport(viewport_off_id)
+    expect(await getViewportCount(session_id)).to.equal(2)
+    // unsubscribe all viewports
+    await unsubscribeViewport(viewport_id)
+    await unsubscribeViewport(viewport_floating_id)
+    await destroyViewport(viewport_id)
+    await destroyViewport(viewport_floating_id)
+    expect(await getViewportCount(session_id)).to.equal(0)
   }).timeout(8000)
 
   it('Should be able to scroll through an editing session', async () => {
     const capacity = 100
+    const desired_viewport_id = 'scroller'
     let viewport_response = await createViewport(
-      'scroller',
+      desired_viewport_id,
       session_id,
       0,
       capacity,
@@ -393,5 +412,12 @@ describe('Viewports', () => {
     expect(dataResponse.getData_asU8()).to.deep.equal(
       Buffer.from('WXYZ/0123456789abcde')
     )
+    expect(await getViewportCount(session_id)).to.equal(1)
+    // Unsubscribe the viewport
+    expect(await unsubscribeViewport(viewport_id)).to.equal(desired_viewport_id)
+    // Note the viewport is not destroyed, just unsubscribed
+    expect(await getViewportCount(session_id)).to.equal(1)
+    await destroyViewport(viewport_id)
+    expect(await getViewportCount(session_id)).to.equal(0)
   }).timeout(8000)
 })

packages/client/webpack.config.js

@@ -20,6 +20,8 @@
 const path = require('path')
 const CopyPlugin = require('copy-webpack-plugin')
 const { PinoWebpackPlugin } = require('pino-webpack-plugin')
+const { CleanWebpackPlugin } = require('clean-webpack-plugin')
+const webpack = require('webpack') // to access built-in plugins
 const fs = require('fs')
 const pkg_version = JSON.parse(fs.readFileSync('./package.json').toString())[
   'version'
@@ -49,6 +51,8 @@ module.exports = {
     ],
   },
   plugins: [
+    new webpack.ProgressPlugin(),
+    new CleanWebpackPlugin(),
     new CopyPlugin({
       patterns: [
         'package.json',

packages/server/src/index.ts

@@ -79,7 +79,7 @@ const getBinFolderPath = (baseDir: string) => {
 async function executeServer(args: string[]): Promise<ChildProcess> {
   const serverScript = path.join(
     getBinFolderPath(path.resolve(__dirname)),
-    os.platform() === 'win32'
+    os.platform() === 'win32' && !process.env.SHELL?.includes('bash')
       ? 'omega-edit-grpc-server.bat'
       : 'omega-edit-grpc-server'
   )

server/scala/README.md

@@ -22,7 +22,7 @@
 - Scala SPI for determing different build info
 - Scala gRPC reference implementation using Apache Pekko.
 
-## Copile and Install
+## Compile and Install
 
 For all of these subsections you will need to be inside of folder `server/scala`
 

server/scala/api/src/main/scala/com/ctc/omega_edit/ChangeImpl.scala

@@ -22,15 +22,25 @@ import jnr.ffi.Pointer
 private[omega_edit] class ChangeImpl(p: Pointer, i: FFI) extends Change {
   require(p != null, "native change pointer was null")
 
-  val id: Long = i.omega_change_get_serial(p)
-
-  val offset: Long = i.omega_change_get_offset(p)
-
-  val length: Long = i.omega_change_get_length(p)
-
-  lazy val data: Array[Byte] = i.omega_change_get_bytes(p).getBytes()
+  lazy val id: Long = i.omega_change_get_serial(p)
+
+  lazy val offset: Long = i.omega_change_get_offset(p)
+
+  lazy val length: Long = i.omega_change_get_length(p)
+
+  lazy val data: Array[Byte] = {
+    val dataPointer = Option(i.omega_change_get_bytes(p))
+    dataPointer match {
+      case Some(pointer) =>
+        val dataArray = new Array[Byte](length.toInt)
+        pointer.get(0, dataArray, 0, length.toInt) // Read the data into the byte array
+        dataArray
+      case None =>
+        throw new IllegalStateException("Data pointer for change is null")
+    }
+  }
 
-  val operation: Change.Op = i.omega_change_get_kind_as_char(p).toChar match {
+  lazy val operation: Change.Op = i.omega_change_get_kind_as_char(p).toChar match {
     case 'D' => Change.Delete
     case 'I' => Change.Insert
     case 'O' => Change.Overwrite