-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update reporter to be compatible with wdio v5 #14
Update reporter to be compatible with wdio v5 #14
Conversation
The reporter is now a class, so it needs to be correctly transpiled for node
Use same engine as wdio
So classes are transpiled correctly
} | ||
} | ||
|
||
module.exports = WdioTeamcityReporter; | ||
module.exports.reporterName = 'teamcity'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like wdio tries to use this as a ES6 module.
Adding module.exports.default = WdioTeamcityReporter;
made it not crash for me, when trying to consume this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just added that. It should be working now. I was using importing the reporter instead of specifying the name so I didn't catch that. Thanks!
bump |
I've been testing with this branch in TeamCity and I'm pleased to say it works but I am seeing some issues:
Have you come across any of these issues while running your test suites on TeamCity? |
@kashinross I think this could be because the plugin is using |
@tagoro9 I gave it a shot with the latest commit and still seeing similar issues - it is behaving slightly differently before but the issues with suites being combined is still happening. Thanks for looking into it! |
A v5 reporter should use |
@christian-bromann That is what I was using initially, but I switched to |
Please let me know if anything is unclear! |
I just dropped the last commit that used |
@tagoro9 Here's a small sample of what's happening The individual tests are titled as |
hi there, what is the status of this pull request ? we are onboarding webdriver-io framework in our company and we need TC reporter support (because currently it just shows that build passed, which is inconvinient, but livable ofcourse). |
Hi, any updates when this will be merged? |
@kashinross Sorry that I didn't reply earlier, it's been some busy weeks. I think can try to dig deeper into your issue this week and see if we can get this to work. (cc @erinev @xemicalas) |
@tagoro9 any luck with that? |
please merge this lads my build log is so messy and I'm having to use custom failure conditions :'( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your work on getting this done!
Regressions
- FlowId is no longer being set
- Stdout is no longer written on a new line, so it's a giant blob of text
Details
I looked into @kashinross's reported bug. It's caused by parallel tests running.
After investigation, flowID is not being set. This is very important for parallel tests, per teamcity documentation. I've made an in-line pr comment to fix.
It's easily replicable having maxInstances set to 2, and using specs:
spec1.js
describe('scenario 1 file1', () => {
it('sc1 1', () => {
browser.pause(500)
});
it('sc1 2', () => {
browser.pause(500)
});
});
describe('scenario 2 file1', () => {
it('sc2 1', () => {
browser.pause(500)
});
it('sc2 2', () => {
browser.pause(500)
});
});
spec2.js:
describe('scenario 3 file2', () => {
it('sc3 1', () => {
browser.pause(500)
});
it('sc3 2', () => {
browser.pause(500)
});
});
wdio v4 teamcity reporter:
##teamcity[testSuiteStarted flowId='5a9958b1-9ebc-f748-a982-1f055bed574a' name='scenario 1 file 1']
wdio v5 teamcity reporter:
##teamcity[testSuiteStarted flowId='' name='scenario 1 file 1']
In addition, I found while the v4 formatter puts each ##teamcity
on a new line, the v5 reporter does not. Made a suggestion in the PR comments.
lib/message.js
Outdated
@@ -20,20 +20,20 @@ const { camelCase, get, invoke } = require('lodash'); | |||
|
|||
const paths = { | |||
browser: ({ suite }) => ['suite', 'runner', suite.cid, 'browserName'], | |||
details: () => 'suite.err.stack', | |||
details: () => 'suite.error.stack', | |||
flowId: ({ suite }) => ['reporter', 'baseReporter', 'stats', 'runners', suite.cid, 'sessionID'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to be updated. I fixed using:
flowId: () => 'reporter.browser.sessionId',
This may also work, based on the console.log output of the ctx object, but I haven't tested
flowId: () => 'reporter.driver.sessionId',
index.js
Outdated
this[event] = (...args) => | ||
flow(buildFormatter(event, opts), msg => { | ||
if (msg) { | ||
this.write(msg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no new line appended to this.write
, so everyone gets dumped as a giant blob with no new lines.
Can you add one via:
this.write(`${msg}\n`)
See example of usage.
So sorry about my delay for looking back at this Thanks for your great feedback @dylanlive! I just pushed some changes addressing it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick update! This is looking much better. While I haven't ran this in TeamCity, looking at the output looks promising
lib/message.js
Outdated
@@ -20,20 +20,20 @@ const { camelCase, get, invoke } = require('lodash'); | |||
|
|||
const paths = { | |||
browser: ({ suite }) => ['suite', 'runner', suite.cid, 'browserName'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might need an update as well. I did a console.log(ctx) to see what object properties are available - you can see the result here
https://gist.github.com/dylanlive/f508a8201f36353954d7e3b03800b297#file-ctx_object-js-L39
Based on that, this works (at least for chrome):
browser: () => 'reporter.browser.capabilities.browserName',
With the latest version of WDIO5 packages |
@kashinross I guess either @sullenor or a contributor to the repo would have to merge this PR and release a new version of the package. |
Thank you for your work. I published the 2.0.0 version with this change. |
This PR updates the reporter to be compatible with wdio v5 following the documentation for creating a custom reporter.
I added the same babel configuration as wdio is using and updated the engines to be at least the same that wdio declares.
The main changes in the reporter are:
write
instead ofconsole.log
Fixes #12