WIP: Safer tests #1946
Closed
WIP: Safer tests #1946
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merged
|
hey @rmg @bajtos I test on one linux vm and manage to reproduce 4 errors.
I fixed them by removing strong-remoting under dependant loopback-connector-remote's folder. When I clone loopback from github and npm install it, loopback-connector-remote installs strong-remoting 2.24.5 automatically under its node_modules folder which causes the failure. I am not sure are they necessarily installed as nested modules of loopback-connector-remote, but from the version number they look old and npm test could pass after I remove them :) Error details: |
|
@jannyHou thank you for the investigation and a detailed report. loopback-connector-remote depends on strong-remoting (and loopback-datasource-juggler) because it uses some of the code/functionality provided by these modules. We were aware that juggler is designed as a singleton (i.e. there should be only one instance of the module in the dependency tree), there is a task to fix it - strongloop/loopback-connector-remote#5 I am quite surprised that strong-remoting suffers from the same problem too. We need to investigate this more to better understand why two instances of strong-remoting don't work well. Here is the reason why this problem is not observed on the 2.x branch: because both |
|
I identified the problem. strong-remoting has a global registry of @jannyHou fixing this problem is non-trivial, I'll work on this myself - see strongloop/loopback-connector-remote#34 |
|
@slnode test please |
|
strongloop/loopback-connector-remote#34 has been landed, let's check how CI likes that |
| @@ -1,4 +1,4 @@ | |||
| # LoopBack | |||
| # LoopBack1 | |||
There was a problem hiding this comment.
Looks like a type/forgotten change, please remove.
If the supertest request fails its basic assertions, there may not even be a body to perform checks against, so bail early when possible.
|
@slnode test please |
|
@slnode test please |
So the pr for "content type" is fine, the error comes from endpoint itself
Detailed error information with |
|
To me, it looks like there is an issue with supertest. I have seen this issue pop up before although I can't remember exactly how it gets resolved. I'm guessing there is an issue booting up the server somewhere. In the past I've seen that for whatever reason, supertest does not "stop" the app instance properly, which would explain why the test is attempted multiple time, and why it passes on it's own but not with the other tests. I'm not familiar with supertest but if @rmg or @bajtos are, maybe you could explain this? In the meantime, I will look into supertest and try to figure out what's going on. |
|
@richardpringle thanks! It makes sense why there are duplicate failures. Yesterday I searched 'Cannot read property 'currentRetry' of undefined' but got few result, while now it's pretty clear. I will look into supertest and see how to solve the conflict. Update: A testing list as this one could pass on all systems. But running For example, test case 'includes loopback.token when necessary'(the first error) will return 401 instead of 200, but when I debug it the @bajtos could you give me some clue about why there is a conflict here? Is it caused by running multiple apps async and expose them to REST? Thanks! Error details: |
This was referenced Feb 1, 2016
Merged
|
Related: #2034 |
|
@bajtos thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I started cleaning up the tests in order to debug the windows failures, but haven't had time to finish yet.
@superkhau and @jannyHou have each contacted me separately about the failures, so I'm opening this PR to get more eyes.