initial test setup

[43081j]

• [ ] CHANGELOG.md has been updated

@TimvdLippe @rictic hows this so far?

i know, a whole load of it is really simple null checks but without looking further into it im not sure how we can test more specifically/deeper.

the travis config is taken from atom's own CI repo with example travis.yml.

p.s. you can run it by going to the directory and atom --test spec

[rictic]

Nice! this is a great start.

I enabled travis for this repo, let's see if this comment kicks it off.

[rictic]

Hm, looks like no. Mind pushing a trivial change up to this branch to kick off the travis run?

[43081j]

@TimvdLippe this is most definitely WIP (i failed to mention that). i was planning on adding tests for the others when i get time (this weekend i imagine).

I will try yarn tonight if i get chance.

[TimvdLippe]

@43081j Great, looking forward to them! Much appreciated :heart:

[43081j]

@TimvdLippe for the travis config, does that mean i need to set the language: node_js?

[43081j]

Unfortunately to make atom's provided build script work, I had to:

1. npm run build
2. rm -r node_modules
3. atom build script

This is because atom's build script its self runs apm clean, which tries to remove node_modules but seems to fail on scoped packages. It then runs apm install, which is the equivalent of doing npm install again.

This is nasty, because we're installing and cleaning twice, essentially. But without taking their build script and keeping a modified copy, I'm not sure what else we can do.

[43081j]

i'm having some real trouble getting the tests to run at all right now, FYI.

for some reason, only one test runs for the linter, then the rest timeout. even if you run only one of the others, they will timeout. will keep investigating

[TimvdLippe]

This weekend I might have time to test it. Are you running the test inside Atom? Because that would probably be easier to run than CI.

On Thu, 16 Mar 2017, 00:04 James Garbutt, notifications@github.com wrote:

i'm having some real trouble getting the tests to run at all right now, FYI.

for some reason, only one test runs for the linter, then the rest timeout. even if you run only one of the others, they will timeout. will keep investigating

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Polymer/atom-plugin/pull/54#issuecomment-286907093, or mute the thread https://github.com/notifications/unsubscribe-auth/AFrDb2sZrQ6xCZru3cAk-N_auVBrSPCmks5rmG56gaJpZM4MdTxu .

[43081j]

Yup, atom --test spec at the min.

It has a really old and/or unusual version of jasmine so it wouldn't surprise me if i just did something wrong in the tests. will try debug

[TimvdLippe]

Filed upstream issue https://github.com/atom/apm/issues/702

[TimvdLippe]

Oh and please enable caching of Yarn per https://blog.travis-ci.com/2016-11-21-travis-ci-now-supports-yarn (see previous review)

[43081j]

Added those changes but the attribute tests currently fail because of the completions coming back empty apparently.

Not entirely sure whats going wrong there...

[TimvdLippe]

I have fixed the attribute tests, but broke others. The reason they were failing, is that the editor service reads from disk. This means that the textbuffer of Atom have to be written to disk too. However, that means we should not overwrite our original fixture. Therefore the solution is to create a temporary directory in which the specs are run. You can see the solution at https://github.com/Polymer/atom-plugin/commits/test-suite

Now for some reason the following errors are thrown:

Autocompleter
  getSuggestions
    it should log an exception if any uncaught
      TypeError: Cannot read property 'editorService' of undefined
        at .<anonymous> (/home/tim/Projects/polymer/atom-plugin/spec/auto-completer-spec.js:42:21)
    it should return nothing if editor service call fails
      Error: Can't save destroyed buffer
        at TextBuffer.module.exports.TextBuffer.saveAs (/usr/share/atom/resources/app.asar/node_modules/text-buffer/lib/text-buffer.js:1054:15)
        at TextBuffer.module.exports.TextBuffer.save (/usr/share/atom/resources/app.asar/node_modules/text-buffer/lib/text-buffer.js:1048:19)
        at TextEditor.module.exports.TextEditor.save (/usr/share/atom/resources/app.asar/src/text-editor.js:915:26)
        at getCompletions (/home/tim/Projects/polymer/atom-plugin/spec/auto-completer-spec.js:15:12)
        at /home/tim/Projects/polymer/atom-plugin/spec/auto-completer-spec.js:68:9
        at .<anonymous> (/usr/share/atom/resources/app.asar/spec/spec-helper.js:354:17)
      Error: Can't save destroyed buffer
        at TextBuffer.module.exports.TextBuffer.saveAs (/usr/share/atom/resources/app.asar/node_modules/text-buffer/lib/text-buffer.js:1054:15)
        at TextBuffer.module.exports.TextBuffer.save (/usr/share/atom/resources/app.asar/node_modules/text-buffer/lib/text-buffer.js:1048:19)
        at TextEditor.module.exports.TextEditor.save (/usr/share/atom/resources/app.asar/src/text-editor.js:915:26)
        at getCompletions (/home/tim/Projects/polymer/atom-plugin/spec/auto-completer-spec.js:15:12)
        at /home/tim/Projects/polymer/atom-plugin/spec/auto-completer-spec.js:68:9
        at .<anonymous> (/usr/share/atom/resources/app.asar/spec/spec-helper.js:354:17)
      timeout: timed out after 5000 msec waiting for promise to be resolved or rejected
    it should suggest matching elements
      timeout: timed out after 5000 msec waiting for promise to be resolved or rejected

But these I can't seem to figure out. Do you have any clue? :joy:

[googlebot]

So there's good news and bad news.

:thumbsup: The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

:confused: The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

[43081j]

FYI @TimvdLippe @rictic, current state:

• rebased to master
• tests for the autocompleter fail (somehow snippets dont get populated or completions aren't found)
• i can't seem to run tests locally anymore, any that try analyzer a file seem to timeout (>5s)

[43081j]

did some debugging, this seems to be an exception which gets eaten: "channel closed".

any idea?

edit:

this seems to fail for me immediately when newing up the editor service, because the first init request to the child process fails with some pipe exception... would explain why my tests always timeout.

[TimvdLippe]

Maybe we are not properly cleaning up the state? Should we explicitly disable the connection in the tearDown?

[TimvdLippe]

Please check out the last 2 commits on the test-suite branch. The tests pass now if you locally use these changes: https://github.com/Polymer/polymer-editor-service/pull/60

:tada: :tada: :tada: :tada: :tada: So happy right now!

[rictic]

stamped a new version of polymer-editor-service!

[43081j]

i bumped the analyzer to 35 and the editor service to the latest version.

[43081j]

it builds!

[TimvdLippe]

:shipit:

[43081j]

we do still need tooltip tests but they seem a little more difficult.

we will need to fire mouse events on the editor and assert that the tooltip element is created, essentially. i can have a look into that this weekend maybe but at least we have tests for the rest now.

[TimvdLippe]

I would say, let's do that in a separate PR. Then we can merge this one to unblock the other open PRs 😄

[43081j]

yup, I agree. this is a pretty major blocker, after all.

:shipit:

[TimvdLippe]

Glad this has been fixed, good work everyone :) I will update all my PRs to include tests, probably this weekend.