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.

Sign up for GitHub

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

Jump to bottom

Get rid of the dependency between TestTools and TestHelpers #2897

Merged
merged 3 commits into from
Nov 21, 2023
Merged

Get rid of the dependency between TestTools and TestHelpers #2897

merged 3 commits into from
Nov 21, 2023

Conversation

testableapple
Copy link
Contributor

@testableapple testableapple commented Nov 17, 2023 •
edited
Loading

🔗 Issue Links

Resolve https://github.com/GetStream/ios-issues-tracking/issues/644

🎯 Goal

  • To improve developer experience, move required stuff from TestHelpers to TestTools

📝 Summary

  • Just moved stuff required by Unit tests from TestHelpers to TestTools

☑️ Contributor Checklist

  • I have signed the Stream CLA (required)
  • This change follows zero ⚠️ policy (required)
  • This change should be manually QAed
  • Changelog is updated with client-facing changes
  • New code is covered by unit tests
  • Comparison screenshots added for visual changes
  • Affected documentation updated (docusaurus, tutorial, CMS)

🎁 Meme

gif

@testableapple testableapple requested a review from a team as a code owner November 17, 2023 12:02
@testableapple testableapple added the 🤖 CI/CD Any work related to CI/CD label Nov 17, 2023
@github-actions GitHub Actions
Copy link

1 Warning
⚠️ Big PR

Generated by 🚫 Danger

martinmitrevski
martinmitrevski reviewed Nov 20, 2023
View reviewed changes
Copy link
Contributor

@martinmitrevski martinmitrevski left a comment

Choose a reason for hiding this comment

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

I think we can also address the Difference/Swifter dependency here.
Apart from that, looks good. I assume customers won't have any build errors while updating to this, right?

Package.swift Outdated
@@ -27,6 +27,10 @@ let package = Package(
targets: ["StreamChatTestMockServer"]
),
],
dependencies: [
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not embed these in the test tools / mock server?

Copy link
Contributor Author

@testableapple testableapple Nov 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Great idea, let's try that. This will increase the PR dramatically, but that will be just embedded libs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@martinmitrevski, embedded 👍

@testableapple testableapple force-pushed the ci/test-tools-dependencies branch from 50ca241 to 8062d8a Compare November 20, 2023 16:01
@sonarqubecloud SonarQubeCloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 44 Code Smells

90.2% 90.2% Coverage
0.1% 0.1% Duplication

martinmitrevski
martinmitrevski approved these changes Nov 21, 2023
View reviewed changes
Copy link
Contributor

@martinmitrevski martinmitrevski left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@testableapple testableapple merged commit 530910b into develop Nov 21, 2023
17 checks passed
@testableapple testableapple deleted the ci/test-tools-dependencies branch November 21, 2023 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinmitrevski martinmitrevski martinmitrevski approved these changes

Assignees
No one assigned
Labels
🤖 CI/CD Any work related to CI/CD
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@testableapple @martinmitrevski