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

[CIVIS-9212] send tests skipped by ITR to the backend #93

Merged
merged 4 commits into from
Mar 28, 2024

Conversation

ypopovych
Copy link
Contributor

What and why?

Added sending of skipped tests to the backend. Fixed span serialisation

How?

  • Added skipped tests sending
  • Cleaned XCTObserver state management
  • Fixed span serialisation

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference

Copy link

@romainkomorn-exdatadog romainkomorn-exdatadog left a comment

Choose a reason for hiding this comment

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

First time I've seen Swift... I think I understand what this is doing, and if I do, then I think it's doing the right thing.

Soooo... get a second opinion. 😄

Copy link
Member

@maciejburda maciejburda left a comment

Choose a reason for hiding this comment

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

Looks good!

Some minor questions / suggestions


func testBundleWillStart(_ testBundle: Bundle) {
let bundleName = testBundle.bundleURL.deletingPathExtension().lastPathComponent
guard case .none = state else {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would make sense to make State implement Equatable and use simple == here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is only one case .none without associated object. I think Equatable implementation will need more lines of code than 2 case unwraps. And this is internal state enum, which is used only in that observer.

Copy link
Member

@maciejburda maciejburda left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for taking the time and answering my questions 🙇

@ypopovych ypopovych merged commit 9d4d09c into main Mar 28, 2024
1 check passed
@ypopovych ypopovych deleted the yehor-popovych/itr-send-skipped branch March 28, 2024 14:02
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.

3 participants