Skip to content

Conversation

@phryneas
Copy link
Contributor

  • do not create a IncrementalChecker instance on the test thread
  • use worker-rpc so the tests can actually access the checker instance run by the service
  • fix include path in "tsconfig-imports.json" so the "should resolve *.vue in the same way as TypeScript" test actually compiles files
  • add two counter checks to make sure the compiler actually throws errors it finds at all

These are essentially all the fixes to the vue tests from #231, including what @johnnyreilly pushed to that branch. I want to get the size of #231 a bit down and having this in master already should be useful.

Thanks to #249 and #246 I could allow the tests to interact with the correct checker instance on the service thread without any further changes to production source code.

Also, this fixes the 'should resolve *.vue in the same way as TypeScript' test, which until now was doing nothing because of a wrong includes path in a tsconfig.json - as the test only checked for 0 errors (and compiling no files creates 0 errors), this was not noticeable.

To prevent such problems in the future, I added two tests that expect an error to be thrown where all parallel tests expect zero errors to be thrown.

Sorry for the chaotic diff - actually there aren't THAT many changes in test/integration/vue.spec.js, but they diff incredibly bad.

- do not create a IncrementalChecker instance on the test thread
- use worker-rpc so the tests can actually access the checker instance run by the service
- fix include path in "tsconfig-imports.json" so the "should resolve *.vue in the same way as TypeScript" test actually compiles files
- add two counter checks to make sure the compiler actually throws errors it finds at all
@johnnyreilly
Copy link
Member

Might be nice to merge this after #251 - could be a nice test of the semantic release mechanism

@phryneas
Copy link
Contributor Author

phryneas commented Apr 20, 2019

Yup, I already did the commit message in the format that would be expected from #251 (I believe) ;)

But please don't merge too much in-between if it could be avoided.

Keeping #231 in sync with this (231 has some minor additions to the test that I kept out of this commit as they do not reflect back on the current master) and #250 on my local dev branch is turning out to be a bit of a nightmare, as I'm kinda three-way-merge-and-cherrypicking right now 😭

@phryneas
Copy link
Contributor Author

phryneas commented Apr 20, 2019

Ah well, #251 has been readied to merge this exact moment so I guess I'll hold off a bit and wait for this to be merged before I do another round of "let's do the dance of the cherry-picks" over in #231 ;)

@phryneas
Copy link
Contributor Author

phryneas commented Apr 20, 2019

I̵ ̵g̵u̵e̵s̵s̵ ̵I̵'̵l̵l̵ ̵r̵e̵o̵p̵e̵n̵ ̵t̵h̵i̵s̵ ̵t̵o̵ ̵b̵e̵ ̵m̵e̵r̵g̵e̵d̵ ̵i̵n̵t̵o̵ ̵̵b̵e̵t̵a̵̵ ̵t̵h̵e̵n̵ ̵:̵)̵

Oh, nice, I can just change the target in this PR :)

@phryneas
Copy link
Contributor Author

@johnnyreilly since #251 should be working now - wanna merge this? :)

@piotr-oles piotr-oles merged commit 5ad2568 into TypeStrong:beta Apr 22, 2019
@piotr-oles
Copy link
Collaborator

🎉 This PR is included in version 1.2.0-beta.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@piotr-oles
Copy link
Collaborator

🎉 This PR is included in version 1.3.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@piotr-oles
Copy link
Collaborator

🎉 This PR is included in version 1.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants