fix(tests): "Unable to read file 's3-read-test:..." #4252

Problem:
Lots of "Unable to read file" noise in the test logs:

    FileViewerManager
    ✔ prompts if file size is greater than 4MB
    ✔ throws if the user cancels a download
    opens text files
    No file system provider found for resource 's3-edit-test:/us-west-2/bucket-name/big-image.jpg'
    ✔ opens a new editor if no document exists
    ✔ closes the read-only tab when opening in edit mode (70ms)
    Unable to read file 's3-read-test:/us-west-2/bucket-name/test1.txt' (Error): Error: Unable to read file 's3-read-test:/us-west-2/bucket-name/test1.txt' (Error)
        at h.doReadFileStream (vscode-file://…)
        at async p.doRead (vscode-file://…)
        at async p.readStream (vscode-file://…)
        at async wr.resolveFromFile (vscode-file://…)
    ✔ re-uses tabs in edit mode when opening as read-only
    ✔ can open in edit mode, showing a warning with two options
    Unable to read file 's3-edit-test:/us-west-2/bucket-name/test1.txt' (Error): Error: Unable to read file 's3-edit-test:/us-west-2/bucket-name/test1.txt' (Error)
        at h.doReadFileStream (vscode-file://…)
        at async p.doRead (vscode-file://…)
        at async p.readStream (vscode-file://…)
        at async wr.resolveFromFile (vscode-file://…)
    ✔ re-uses an editor if already opened, focusing it (75ms)
    Unable to read file 's3-read-test:/us-west-2/bucket-name/test1.txt' (Error): Error: Unable to read file 's3-read-test:/us-west-2/bucket-name/test1.txt' (Error)
        at h.doReadFileStream (vscode-file://…)
        at async p.doRead (vscode-file://…)
        at async p.readStream (vscode-file://…)
        at async wr.resolveFromFile (vscode-file://…)

Solution:
- Use one VirualFileSystem provider instead of re-creating for each test.
- Use `fileViewerManager.closeEditors()` instead of `closeAllEditors()`
  (which is unreliable).
- Remove the "case-insensitive" test because it's a vscode feature, we
  don't enhance it.

Author: justinmk3

src/s3/commands/openFile.ts

@@ -6,19 +6,25 @@
 import * as vscode from 'vscode'
 import { localize } from '../../shared/utilities/vsCodeUtils'
 import { S3FileNode } from '../explorer/s3FileNode'
-import { S3FileViewerManager } from '../fileViewerManager'
+import { S3FileViewerManager, S3Tab } from '../fileViewerManager'
 import { downloadFileAsCommand } from './downloadFileAs'
 import { telemetry } from '../../shared/telemetry/telemetry'
 
 const sizeLimit = 50 * Math.pow(10, 6)
 
-export async function openFileReadModeCommand(node: S3FileNode, manager: S3FileViewerManager): Promise<void> {
+export async function openFileReadModeCommand(
+    node: S3FileNode,
+    manager: S3FileViewerManager
+): Promise<S3Tab | undefined> {
     if (await isFileSizeValid(node.file.sizeBytes, node)) {
         return telemetry.s3_openEditor.run(() => manager.openInReadMode({ bucket: node.bucket, ...node.file }))
     }
 }
 
-export async function editFileCommand(uriOrNode: vscode.Uri | S3FileNode, manager: S3FileViewerManager): Promise<void> {
+export async function editFileCommand(
+    uriOrNode: vscode.Uri | S3FileNode,
+    manager: S3FileViewerManager
+): Promise<S3Tab | undefined> {
     if (uriOrNode instanceof S3FileNode) {
         const size = uriOrNode.file.sizeBytes
 

src/s3/fileViewerManager.ts

@@ -130,9 +130,12 @@ export class S3FileViewerManager {
         this.disposables.push(this.registerTabCleanup())
     }
 
-    /**
-     * Removes all active editors as well as any underlying files
-     */
+    /** Disposes all active editors and underlying files. */
+    public async closeEditors(): Promise<void> {
+        await Promise.all([...Object.values(this.activeTabs).map(v => v?.dispose())])
+    }
+
+    /** Disposes all active editors, underlying files, providers, and other resources. */
     public async dispose(): Promise<void> {
         await Promise.all([
             ...Object.values(this.activeTabs).map(v => v?.dispose()),
@@ -177,8 +180,12 @@ export class S3FileViewerManager {
 
     private async closeEditor(editor: vscode.TextEditor | undefined): Promise<void> {
         if (editor && !editor.document.isClosed) {
-            await vscode.window.showTextDocument(editor.document, { preserveFocus: false })
-            await vscode.commands.executeCommand('workbench.action.closeActiveEditor')
+            await vscode.window.showTextDocument(editor.document, { preserveFocus: false }).then(
+                r => vscode.commands.executeCommand('workbench.action.closeActiveEditor'),
+                e => {
+                    getLogger().warn('S3FileViewer: showTextDocument failed to open: "%s"', editor.document.uri)
+                }
+            )
         }
     }
 
@@ -209,7 +216,7 @@ export class S3FileViewerManager {
      *
      * @param file
      */
-    public async openInReadMode(file: S3File): Promise<void> {
+    public async openInReadMode(file: S3File): Promise<S3Tab | undefined> {
         const contentType = mime.contentType(path.extname(file.name))
         const isTextDocument = contentType && mime.charset(contentType) === 'UTF-8'
 
@@ -223,7 +230,7 @@ export class S3FileViewerManager {
             return this.openInEditMode(file)
         }
 
-        await this.createTab(file, TabMode.Read)
+        return this.createTab(file, TabMode.Read)
     }
 
     /**
@@ -232,7 +239,7 @@ export class S3FileViewerManager {
      *
      * @param uriOrFile to be opened
      */
-    public async openInEditMode(uriOrFile: vscode.Uri | S3File): Promise<void> {
+    public async openInEditMode(uriOrFile: vscode.Uri | S3File): Promise<S3Tab | undefined> {
         const uri = uriOrFile instanceof vscode.Uri ? uriOrFile : this.fileToUri(uriOrFile, TabMode.Edit)
         const activeTab = await this.tryFocusTab(uri, uri.with({ scheme: this.schemes.read }))
         const file = activeTab?.file ?? uriOrFile
@@ -248,7 +255,7 @@ export class S3FileViewerManager {
         await activeTab?.dispose()
         void this.showEditNotification()
 
-        await this.createTab(file, TabMode.Edit)
+        return this.createTab(file, TabMode.Edit)
     }
 
     private registerProvider(file: S3File, uri: vscode.Uri): vscode.Disposable {
@@ -266,7 +273,7 @@ export class S3FileViewerManager {
     /**
      * Creates a new tab based on the mode
      */
-    private async createTab(file: S3File, mode: TabMode): Promise<void> {
+    private async createTab(file: S3File, mode: TabMode): Promise<S3Tab | undefined> {
         if (!(await this.canContinueDownload(file))) {
             throw new CancellationError('user')
         }
@@ -291,6 +298,8 @@ export class S3FileViewerManager {
                 }
             },
         }
+
+        return this.activeTabs[key]
     }
 
     private async canContinueDownload(file: S3File): Promise<boolean> {

src/test/s3/util/fileViewerManager.test.ts

@@ -13,7 +13,7 @@ import { VirtualFileSystem } from '../../../shared/virtualFilesystem'
 import { bufferToStream } from '../../../shared/utilities/streamUtilities'
 import { MockOutputChannel } from '../../mockOutputChannel'
 import { SeverityLevel } from '../../shared/vscode/message'
-import { assertTelemetry, assertTextEditorContains, closeAllEditors } from '../../testUtil'
+import { assertTelemetry, assertTextEditorContains } from '../../testUtil'
 import { PromptSettings } from '../../../shared/settings'
 import { stub } from '../../utilities/stubber'
 import { assertHasProps } from '../../../shared/utilities/tsUtils'
@@ -187,14 +187,14 @@ describe('FileViewerManager', function () {
         return window.visibleTextEditors.filter(e => e.document.fileName.endsWith(documentName))
     }
 
-    function registerFileSystemProviders(isCaseSensitive: boolean = true): vscode.Disposable[] {
+    function registerFileSystemProviders(): vscode.Disposable[] {
         return [
-            vscode.workspace.registerFileSystemProvider(editScheme, fs, { isCaseSensitive }),
-            vscode.workspace.registerFileSystemProvider(readScheme, fs, { isReadonly: true, isCaseSensitive }),
+            vscode.workspace.registerFileSystemProvider(editScheme, fs, { isCaseSensitive: true }),
+            vscode.workspace.registerFileSystemProvider(readScheme, fs, { isReadonly: true, isCaseSensitive: true }),
         ]
     }
 
-    beforeEach(function () {
+    before(function () {
         s3 = createS3()
         fs = new VirtualFileSystem()
 
@@ -207,18 +207,25 @@ describe('FileViewerManager', function () {
     })
 
     afterEach(async function () {
-        await closeAllEditors()
-        vscode.Disposable.from(...disposables).dispose()
+        await fileViewerManager.closeEditors()
+    })
+
+    after(async function () {
+        await vscode.Disposable.from(...disposables).dispose()
+        await fileViewerManager.dispose()
     })
 
     it('prompts if file size is greater than 4MB', async function () {
-        void fileViewerManager.openInReadMode({ ...bigImage, bucket })
+        // User can "Continue".
+        const didOpen = fileViewerManager.openInReadMode({ ...bigImage, bucket })
         await getTestWindow()
             .waitForMessage(/File size is more than 4MB/)
             .then(message => message.selectItem(/Continue/))
+        await (await didOpen)?.dispose()
     })
 
     it('throws if the user cancels a download', async function () {
+        // User can "Cancel".
         const didOpen = fileViewerManager.openInReadMode({ ...bigImage, bucket })
         await getTestWindow()
             .waitForMessage(/File size is more than 4MB/)
@@ -232,7 +239,7 @@ describe('FileViewerManager', function () {
         const textFile2Contents = Buffer.from('test2 contents', 'utf-8')
         const textFile2 = makeFile('test2.txt', textFile2Contents)
 
-        beforeEach(function () {
+        before(function () {
             s3.addFile(textFile1)
             s3.addFile(textFile2)
         })
@@ -324,30 +331,4 @@ describe('FileViewerManager', function () {
             await assertTextEditorContains(upperCaseFileContent)
         })
     })
-
-    describe('file uri case insensitivity', function () {
-        beforeEach(function () {
-            vscode.Disposable.from(...disposables).dispose()
-            // Case insensitive file system providers
-            disposables = registerFileSystemProviders(false)
-        })
-
-        it('opens an existing file since names are the same', async function () {
-            // Create and assert content of file with lowercase name
-            const lowerCaseFileContent = 'lowercaseContent'
-            const lowerCaseFile = makeFile('file.txt', Buffer.from(lowerCaseFileContent, 'utf-8'))
-            s3.addFile(lowerCaseFile)
-            await fileViewerManager.openInReadMode({ ...lowerCaseFile, bucket })
-            await assertTextEditorContains(lowerCaseFileContent)
-
-            // Create similarily named file, but with uppercase character
-            const upperCaseFileContent = 'uppercaseContent'
-            const upperCaseFile = makeFile('File.txt', Buffer.from(upperCaseFileContent, 'utf-8'))
-            s3.addFile(upperCaseFile)
-
-            // Attempt to open uppercase file opens existing lowercase file instead
-            await fileViewerManager.openInReadMode({ ...upperCaseFile, bucket })
-            await assertTextEditorContains(lowerCaseFileContent)
-        })
-    })
 })