Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Update pathwatcher to 6.x #99

Merged
merged 11 commits into from
Sep 22, 2015
Merged

Update pathwatcher to 6.x #99

merged 11 commits into from
Sep 22, 2015

Conversation

winstliu
Copy link
Contributor

Should allow text-buffer to compile on Node 4.

Closes #98

@mnquintana
Copy link
Contributor

I'm wondering if we should add Node 4.x to the build matrix maybe (for our Node libraries)? (so we don't necessarily have to rely on user reports)

/cc @zcbenz @atom/feedback

@winstliu
Copy link
Contributor Author

script/cibuild still uses Node 0.12 though - should I also update that to Node 4?
EDIT: Ninja'd 😆

@zcbenz
Copy link
Contributor

zcbenz commented Sep 14, 2015

I'm wondering if we should add Node 4.x to the build matrix maybe (for our Node libraries)? (so we don't necessarily have to rely on user reports)

Sounds good to me.

@winstliu
Copy link
Contributor Author

Not sure where to proceed from here...Clang actually seems to be segfaulting when running under Node 4. There's also a lot of failures related to temp files.

@YurySolovyov
Copy link

@50Wliu segfaults may gone after npm rebuild or just rm -rf node_modules + npm install

@winstliu
Copy link
Contributor Author

Once I get atom/node-pathwatcher#90 all set up and merged, then I'll rebase/update this PR accordingly.

@winstliu winstliu removed the blocked label Sep 18, 2015
@winstliu
Copy link
Contributor Author

Tests continue to fail but the install no longer segfaults. npm install also fails on the first run, just like atom/node-pathwatcher (but this time with more errors).
Looking into the spec failures now...

@winstliu
Copy link
Contributor Author

Ok so I've fixed most of the errors by simply updating temp but there are still three failures related to temp file modification that I don't know how to solve:

TextBuffer
  serialization
    when the buffer has a path
      when the serialized buffer had no unsaved changes
        it loads the current contents of the file at the serialized path
          Expected 'words!' to be 'words'. (spec/text-buffer-spec.coffee:913:39)
  ::onDidChangePath()
    it notifies observers when the buffer's file is moved
      timeout: timed out after 5000 msec waiting for buffer path change
  when the buffer's file is deleted (via another process)
    when the file is not modified
      it retains its path and reports the buffer as not modified
        Expected true to be falsy. (spec/text-buffer-spec.coffee:1169:45)

@zcbenz
Copy link
Contributor

zcbenz commented Sep 21, 2015

It seems that buffer.testSerialization doesn't keep the undo stack but the test assumes so, I'm not sure of the expected behavior, /cc @maxbrunsfeld for help.

@maxbrunsfeld
Copy link
Contributor

@zcbenz I think this is due to a pathwatcher bug that I had fixed on the four and five branches, but I neglected to fix on master. I have merged my fix into master and upgraded pathwatcher on this branch.

@zcbenz
Copy link
Contributor

zcbenz commented Sep 22, 2015

@maxbrunsfeld Thanks!

For the rest of the failing specs, it is actually caused by switching to Linux in Travis CI, it seems that they never passed on Linux in previous versions.

I'm going to suppress those specs for now so we can focus on updating to latest Electron in Atom.

zcbenz added a commit that referenced this pull request Sep 22, 2015
@zcbenz zcbenz merged commit 55818c4 into master Sep 22, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants