From 50f02495c0995ba84245f6156df200dbfe2875b9 Mon Sep 17 00:00:00 2001
From: Jason Rudolph <jason@jasonrudolph.com>
Date: Fri, 4 Aug 2017 14:28:28 -0400
Subject: [PATCH] Attempt to fix flaky test re: flashing highlight decorations

Based on the assertion failures seen in
https://github.com/atom/atom/issues/15158#issue-247808059, it seems that
the flash for class 'c' sometimes ends before the flash for class 'd'
happens. Prior to this change, we only flashed class 'c' for 100ms, and
perhaps that isn't always enough time.

In this commit, we increase the flash duration from 100ms to 1000ms,
greatly increasing the likelihood that we're allowing enough time for
the flash on class 'd' to take place before the flash for class 'c'
ends. We also extract the scenario into its own test, so that 1) we can
more clearly explain the scenario that these assertions are testing and
2) future intermittent test failures will be easier to isolate.
---
 spec/text-editor-component-spec.js | 42 +++++++++++++++---------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/spec/text-editor-component-spec.js b/spec/text-editor-component-spec.js
index 481602871..ad631b855 100644
--- a/spec/text-editor-component-spec.js
+++ b/spec/text-editor-component-spec.js
@@ -1443,27 +1443,6 @@ describe('TextEditorComponent', () => {
       expect(highlights[0].classList.contains('b')).toBe(false)
       expect(highlights[1].classList.contains('b')).toBe(false)
 
-      // Flash existing highlight
-      decoration.flash('c', 100)
-      await component.getNextUpdatePromise()
-      expect(highlights[0].classList.contains('c')).toBe(true)
-      expect(highlights[1].classList.contains('c')).toBe(true)
-
-      // Add second flash class
-      decoration.flash('d', 100)
-      await component.getNextUpdatePromise()
-      expect(highlights[0].classList.contains('c')).toBe(true)
-      expect(highlights[1].classList.contains('c')).toBe(true)
-      expect(highlights[0].classList.contains('d')).toBe(true)
-      expect(highlights[1].classList.contains('d')).toBe(true)
-
-      await conditionPromise(() =>
-        !highlights[0].classList.contains('c') &&
-        !highlights[1].classList.contains('c') &&
-        !highlights[0].classList.contains('d') &&
-        !highlights[1].classList.contains('d')
-      )
-
       // Flashing the same class again before the first flash completes
       // removes the flash class and adds it back on the next frame to ensure
       // CSS transitions apply to the second flash.
@@ -1488,6 +1467,27 @@ describe('TextEditorComponent', () => {
       )
     })
 
+    it("flashing a highlight decoration doesn't unflash other highlight decorations", async () => {
+      const {component, element, editor} = buildComponent({rowsPerTile: 3, height: 200})
+      const marker = editor.markScreenRange([[2, 4], [3, 4]])
+      const decoration = editor.decorateMarker(marker, {type: 'highlight', class: 'a'})
+
+      // Flash one class
+      decoration.flash('c', 1000)
+      await component.getNextUpdatePromise()
+      const highlights = element.querySelectorAll('.highlight.a')
+      expect(highlights[0].classList.contains('c')).toBe(true)
+      expect(highlights[1].classList.contains('c')).toBe(true)
+
+      // Flash another class while the previously-flashed class is still highlighted
+      decoration.flash('d', 100)
+      await component.getNextUpdatePromise()
+      expect(highlights[0].classList.contains('c')).toBe(true)
+      expect(highlights[1].classList.contains('c')).toBe(true)
+      expect(highlights[0].classList.contains('d')).toBe(true)
+      expect(highlights[1].classList.contains('d')).toBe(true)
+    })
+
     it('supports layer decorations', async () => {
       const {component, element, editor} = buildComponent({rowsPerTile: 12})
       const markerLayer = editor.addMarkerLayer()