add custom window constructor#126
add custom window constructor#126idanrozin wants to merge 3 commits intoGoogleCloudPlatform:masterfrom
Conversation
| errorHandler.start({key: 'key', projectId: 'projectId'}); | ||
| expect(errorHandler.serviceContext.service).to.equal('web'); | ||
| }); | ||
| function initialization() { |
There was a problem hiding this comment.
This whole test file has been substantially re-written.
Can you share why this was needed?
To minimize risk of merging this PR, it would be best if this file:
- was modified as little as possible
- was kept as simple as possible (avoid having function that returns a function)
If you'd like to refactor this test file, it's best to do it in a separate PR
There was a problem hiding this comment.
Thanks, as I mentioned in the description of this PR, I'm not sure why, but in the git diff, it seems that I made a lot of changes to the tests.
But, I did not really changed much other than refactoring anonymous functions to named functions and then used them in the base cases and re-used them in the new test cases that are the same, but with a different object passed to the constructor.
Due to how mocha is built, I needed some of the functions to return a function because I needed to pass extra data and I didn't want to duplicate the same functions.
But if this concerns you, I can leave it as it was, and just duplicate some of the code which I think that is needed to test for specific test cases and that's it. I'm fine with whatever you decide.
There was a problem hiding this comment.
Thanks.
Yes, when it comes to tests, I prefer to minimize changes and refactoring, and to duplicate code if needed.
3 participants
So, this PR is related to issue #124 which I have opened.
This PR is for whomever is interested. Hope it helps and someone will take it to review and merge :)
Regarding the tests. I know it looks awful in the diff view, but what I essentially did is: I just created another test suite which runs the same tests but with the new constructor addition. So i needed to extract the functions logic outside and use closure for some functions.
But basically this is the new structure of the tests:
Before each and after each are the same except for the instantiation of
StackdriverErrorReporterandwindowwhich is done inside the tests suites itself.first suite -> same logic
second suite -> same logic just with
new StackdriverErrorReporter(customWindow)Thanks!