Skip to content
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

Passing configuration options to reporter (closes #5088) #5096

Merged
merged 3 commits into from May 28, 2020

Conversation

wentwrong
Copy link
Contributor

@wentwrong wentwrong commented May 19, 2020

Purpose

Having configuration options allows the reporter to provide additional info

Approach

Feeding options as a param to reportTaskStart method

References

#5088 (options have screenshot and video dir paths)
#3497 (screenshots/videos paths can be relativized)

Pre-Merge TODO

  • Write tests for your proposed changes
  • Make sure that existing tests do not fail

@need-response-app need-response-app bot added the STATE: Need response An issue that requires a response or attention from the team. label May 19, 2020
@testcafe-build-bot
Copy link
Collaborator

@need-response-app need-response-app bot removed the STATE: Need response An issue that requires a response or attention from the team. label May 19, 2020
@wentwrong wentwrong marked this pull request as draft May 20, 2020 07:52
@testcafe-build-bot
Copy link
Collaborator

@testcafe-build-bot
Copy link
Collaborator

@testcafe-build-bot
Copy link
Collaborator

@wentwrong wentwrong marked this pull request as ready for review May 21, 2020 11:52
@@ -172,7 +172,7 @@ export default class Reporter {
const userAgents = task.browserConnectionGroups.map(group => group[0].userAgent);
const first = this.reportQueue[0];

await this.plugin.reportTaskStart(startTime, userAgents, this.testCount, task.testStructure);
await this.plugin.reportTaskStart(startTime, userAgents, this.testCount, task.testStructure, task.opts);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to pass an object with the configuration field, not a configuration object: ``. It will help us prevent breaking changes if we need to add a new argument in the future.

@wentwrong wentwrong marked this pull request as draft May 27, 2020 10:03
@testcafe-build-bot
Copy link
Collaborator

@wentwrong wentwrong marked this pull request as ready for review May 27, 2020 14:08
@AndreyBelym AndreyBelym merged commit 64cf617 into DevExpress:master May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants