Merge pull request #1903 from atom/aw/coverage/misc

Correct remaining flapping test coverage

Author: smashwilson

lib/atom/decoration.js

@@ -64,12 +64,12 @@ class BareDecoration extends React.Component {
   componentDidUpdate(prevProps) {
     if (this.props.editorHolder !== prevProps.editorHolder) {
       this.editorSub.dispose();
-      this.editorSub = this.state.editorHolder.observe(this.observeParents);
+      this.editorSub = this.props.editorHolder.observe(this.observeParents);
     }
 
     if (this.props.decorableHolder !== prevProps.decorableHolder) {
       this.decorableSub.dispose();
-      this.decorableSub = this.state.decorableHolder.observe(this.observeParents);
+      this.decorableSub = this.props.decorableHolder.observe(this.observeParents);
     }
 
     if (
@@ -95,10 +95,10 @@ class BareDecoration extends React.Component {
     this.decorationHolder.map(decoration => decoration.destroy());
 
     const editorValid = this.props.editorHolder.map(editor => !editor.isDestroyed()).getOr(false);
-    const markableValid = this.props.decorableHolder.map(decorable => !decorable.isDestroyed()).getOr(false);
+    const decorableValid = this.props.decorableHolder.map(decorable => !decorable.isDestroyed()).getOr(false);
 
     // Ensure the Marker or MarkerLayer corresponds to the context's TextEditor
-    const markableMatches = this.props.decorableHolder.map(decorable => this.props.editorHolder.map(editor => {
+    const decorableMatches = this.props.decorableHolder.map(decorable => this.props.editorHolder.map(editor => {
       const layer = decorable.layer || decorable;
       const displayLayer = editor.getMarkerLayer(layer.id);
       if (!displayLayer) {
@@ -110,7 +110,7 @@ class BareDecoration extends React.Component {
       return true;
     }).getOr(false)).getOr(false);
 
-    if (!editorValid || !markableValid || !markableMatches) {
+    if (!editorValid || !decorableValid || !decorableMatches) {
       return;
     }
 

lib/controllers/editor-conflict-controller.js

@@ -18,11 +18,7 @@ export default class EditorConflictController extends React.Component {
     commandRegistry: PropTypes.object.isRequired,
     resolutionProgress: PropTypes.object.isRequired,
     isRebase: PropTypes.bool.isRequired,
-    refreshResolutionProgress: PropTypes.func,
-  }
-
-  static defaultProps = {
-    refreshResolutionProgress: () => {},
+    refreshResolutionProgress: PropTypes.func.isRequired,
   }
 
   constructor(props, context) {
@@ -49,7 +45,6 @@ export default class EditorConflictController extends React.Component {
     const buffer = this.props.editor.getBuffer();
 
     this.subscriptions.add(
-      this.props.editor.onDidStopChanging(() => this.forceUpdate()),
       this.props.editor.onDidDestroy(() => this.props.refreshResolutionProgress(this.props.editor.getPath())),
       buffer.onDidReload(() => this.reparseConflicts()),
     );
@@ -171,7 +166,7 @@ export default class EditorConflictController extends React.Component {
   }
 
   dismissConflicts(conflicts) {
-    this.setState((prevState, props) => {
+    this.setState(prevState => {
       const {added} = compareSets(new Set(conflicts), prevState.conflicts);
       return {conflicts: added};
     });

lib/models/conflicts/banner.js

@@ -26,6 +26,7 @@ export default class Banner {
   revert() {
     const range = this.getMarker().getBufferRange();
     this.editor.setTextInBufferRange(range, this.originalText);
+    this.getMarker().setBufferRange(range);
   }
 
   delete() {

lib/models/conflicts/side.js

@@ -98,6 +98,7 @@ export default class Side {
   revert() {
     const range = this.getMarker().getBufferRange();
     this.editor.setTextInBufferRange(range, this.originalText);
+    this.getMarker().setBufferRange(range);
   }
 
   deleteBanner() {

lib/models/repository-states/present.js

@@ -610,9 +610,6 @@ export default class Present extends State {
         const bundle = await this.git().getStatusBundle();
         const results = await this.formatChangedFiles(bundle);
         results.branch = bundle.branch;
-        if (!results.branch.aheadBehind) {
-          results.branch.aheadBehind = {ahead: null, behind: null};
-        }
         return results;
       } catch (err) {
         if (err instanceof LargeRepoError) {
@@ -855,7 +852,7 @@ export default class Present extends State {
   // Direct blob access
 
   getBlobContents(sha) {
-    return this.cache.getOrSet(Keys.blob(sha), () => {
+    return this.cache.getOrSet(Keys.blob.oneWith(sha), () => {
       return this.git().getBlobContents(sha);
     });
   }
@@ -876,6 +873,7 @@ export default class Present extends State {
 
   // Cache
 
+  /* istanbul ignore next */
   getCache() {
     return this.cache;
   }
@@ -974,6 +972,7 @@ class Cache {
     this.didUpdate();
   }
 
+  /* istanbul ignore next */
   [Symbol.iterator]() {
     return this.storage[Symbol.iterator]();
   }
@@ -988,6 +987,7 @@ class Cache {
     this.emitter.emit('did-update');
   }
 
+  /* istanbul ignore next */
   onDidUpdate(callback) {
     return this.emitter.on('did-update', callback);
   }
@@ -1025,6 +1025,7 @@ class CacheKey {
     }
   }
 
+  /* istanbul ignore next */
   toString() {
     return `CacheKey(${this.primary})`;
   }
@@ -1041,6 +1042,7 @@ class GroupKey {
     }
   }
 
+  /* istanbul ignore next */
   toString() {
     return `GroupKey(${this.group})`;
   }

test/atom/decoration.test.js

@@ -99,6 +99,69 @@ describe('Decoration', function() {
     });
   });
 
+  describe('when called with changed props', function() {
+    let wrapper, originalDecoration;
+
+    beforeEach(function() {
+      const app = (
+        <Decoration
+          editor={editor}
+          decorable={marker}
+          type="line"
+          position="head"
+          className="something"
+        />
+      );
+      wrapper = mount(app);
+
+      originalDecoration = editor.getLineDecorations({position: 'head', class: 'something'})[0];
+    });
+
+    it('redecorates when a new Editor and Marker are provided', async function() {
+      const newEditor = await workspace.open(require.resolve('../../package.json'));
+      const newMarker = newEditor.markBufferRange([[0, 0], [2, 0]]);
+
+      wrapper.setProps({editor: newEditor, decorable: newMarker});
+
+      assert.isTrue(originalDecoration.isDestroyed());
+      assert.lengthOf(editor.getLineDecorations({position: 'head', class: 'something'}), 0);
+      assert.lengthOf(newEditor.getLineDecorations({position: 'head', class: 'something'}), 1);
+    });
+
+    it('redecorates when a new MarkerLayer is provided', function() {
+      const newLayer = editor.addMarkerLayer();
+
+      wrapper.setProps({decorable: newLayer, decorateMethod: 'decorateMarkerLayer'});
+
+      assert.isTrue(originalDecoration.isDestroyed());
+      assert.lengthOf(editor.getLineDecorations({position: 'head', class: 'something'}), 0);
+
+      // Turns out Atom doesn't provide any public way to query the markers on a layer.
+      assert.lengthOf(
+        Array.from(editor.decorationManager.layerDecorationsByMarkerLayer.get(newLayer)),
+        1,
+      );
+    });
+
+    it('updates decoration properties', function() {
+      wrapper.setProps({className: 'different'});
+
+      assert.lengthOf(editor.getLineDecorations({position: 'head', class: 'something'}), 0);
+      assert.lengthOf(editor.getLineDecorations({position: 'head', class: 'different'}), 1);
+      assert.isFalse(originalDecoration.isDestroyed());
+      assert.strictEqual(originalDecoration.getProperties().class, 'different');
+    });
+
+    it('does not redecorate when the decorable is on the wrong TextEditor', async function() {
+      const newEditor = await workspace.open(require.resolve('../../package.json'));
+
+      wrapper.setProps({editor: newEditor});
+
+      assert.isTrue(originalDecoration.isDestroyed());
+      assert.lengthOf(editor.getLineDecorations({}), 0);
+    });
+  });
+
   it('destroys its decoration on unmount', function() {
     const app = (
       <Decoration

test/controllers/editor-conflict-controller.test.js

@@ -166,6 +166,49 @@ describe('EditorConflictController', function() {
       assert.strictEqual(conflicts[2].getChosenSide(), conflicts[2].getSide(THEIRS));
     });
 
+    it('resolves multiple conflicts as "ours"', function() {
+      assert.isFalse(conflicts[0].isResolved());
+      assert.isFalse(conflicts[1].isResolved());
+      assert.isFalse(conflicts[2].isResolved());
+
+      editor.setCursorBufferPosition([8, 3]); // On "Your changes"
+      editor.addCursorAtBufferPosition([11, 2]); // On "Text in between 0 and 1."
+      editor.addCursorAtBufferPosition([14, 5]); // On "My middle changes"
+      editor.addCursorAtBufferPosition([15, 0]); // On "======="
+      commandRegistry.dispatch(editorView, 'github:resolve-as-ours');
+
+      assert.isTrue(conflicts[0].isResolved());
+      assert.strictEqual(conflicts[0].getChosenSide(), conflicts[0].getSide(OURS));
+      assert.deepEqual(conflicts[0].getUnchosenSides(), [conflicts[0].getSide(THEIRS)]);
+
+      assert.isTrue(conflicts[1].isResolved());
+      assert.strictEqual(conflicts[1].getChosenSide(), conflicts[1].getSide(OURS));
+      assert.deepEqual(conflicts[1].getUnchosenSides(), [conflicts[1].getSide(THEIRS)]);
+
+      assert.isFalse(conflicts[2].isResolved());
+    });
+
+    it('resolves multiple conflicts as "theirs"', function() {
+      assert.isFalse(conflicts[0].isResolved());
+      assert.isFalse(conflicts[1].isResolved());
+      assert.isFalse(conflicts[2].isResolved());
+
+      editor.setCursorBufferPosition([8, 3]); // On "Your changes"
+      editor.addCursorAtBufferPosition([11, 2]); // On "Text in between 0 and 1."
+      editor.addCursorAtBufferPosition([22, 5]); // On "More of my changes"
+      commandRegistry.dispatch(editorView, 'github:resolve-as-theirs');
+
+      assert.isTrue(conflicts[0].isResolved());
+      assert.strictEqual(conflicts[0].getChosenSide(), conflicts[0].getSide(THEIRS));
+      assert.deepEqual(conflicts[0].getUnchosenSides(), [conflicts[0].getSide(OURS)]);
+
+      assert.isFalse(conflicts[1].isResolved());
+
+      assert.isTrue(conflicts[2].isResolved());
+      assert.strictEqual(conflicts[2].getChosenSide(), conflicts[2].getSide(THEIRS));
+      assert.deepEqual(conflicts[2].getUnchosenSides(), [conflicts[2].getSide(OURS)]);
+    });
+
     it('disregards conflicts with cursors on both sides', function() {
       editor.setCursorBufferPosition([6, 3]); // On "Multi-line even"
       editor.addCursorAtBufferPosition([14, 1]); // On "My middle changes"
@@ -316,6 +359,26 @@ describe('EditorConflictController', function() {
 
       await assert.async.isTrue(refreshResolutionProgress.calledWith(fixtureFile));
     });
+
+    it('performs a resolution from the context menu', function() {
+      const conflict = conflicts[1];
+      assert.isFalse(conflict.isResolved());
+
+      wrapper.find('ConflictController').at(1).prop('resolveAsSequence')([OURS]);
+
+      assert.isTrue(conflict.isResolved());
+      assert.strictEqual(conflict.getChosenSide(), conflict.getSide(OURS));
+    });
+
+    it('dismisses a conflict from the context menu', function() {
+      const conflict = conflicts[2];
+
+      wrapper.find('ConflictController').at(2).prop('dismiss')();
+      wrapper.update();
+
+      assert.lengthOf(wrapper.find(ConflictController), 2);
+      assert.isFalse(wrapper.find(ConflictController).someWhere(cc => cc.prop('conflict') === conflict));
+    });
   });
 
   describe('on a file with 3-way diff markers', function() {
@@ -442,4 +505,12 @@ describe('EditorConflictController', function() {
       assert.equal(editor.getText(), 'These are my changes\nThese are your changes\n\nPast the end\n');
     });
   });
+
+  it('cleans up its subscriptions when unmounting', async function() {
+    await useFixture('triple-2way-diff.txt');
+    wrapper.unmount();
+
+    editor.destroy();
+    assert.isFalse(refreshResolutionProgress.called);
+  });
 });

test/fixtures/conflict-marker-examples/single-2way-diff-empty.txt

@@ -0,0 +1,8 @@
+Before the start
+
+<<<<<<< HEAD
+These are my changes
+=======
+>>>>>>> master
+
+Past the end