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

feat: Adding unit test for Slack API interactions using testify/mock #26

Merged
merged 14 commits into from
Apr 18, 2023

Conversation

JoseAngel1196
Copy link
Contributor

@JoseAngel1196 JoseAngel1196 commented Apr 14, 2023

Changes Made

In this pull request, I have added unit tests for Slack API using testify/mock.

Description

Previously, we were facing difficulties in adding unit tests due to tight coupling with the Slack API, which made it challenging to mock and test our code independently.

To address this issue, I have implemented encapsulation by defining an interface for the Slack API and implementing its methods by referencing the external package. Now, our package consumers (e.g., scan.go) can use the interface without needing to directly import the 'go-path/to/external' package, which helps decouple our code from the external dependency. Additionally, I have introduced a factory method to abstract the implementation details of the Slack Client.

With these changes, we can now easily mock the Slack API interface for unit testing, improving the testability and maintainability of our code.

@codecov
Copy link

codecov bot commented Apr 14, 2023

Codecov Report

Merging #26 (8f287cd) into main (038dcbf) will increase coverage by 10.10%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##            main      #26       +/-   ##
==========================================
+ Coverage   6.60%   16.71%   +10.10%     
==========================================
  Files          9        9               
  Lines        348      347        -1     
==========================================
+ Hits          23       58       +35     
+ Misses       325      288       -37     
- Partials       0        1        +1     
Flag Coverage Δ
unittests 16.71% <66.66%> (+10.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
internal/scan.go 0.00% <0.00%> (ø)
api/slack.go 100.00% <100.00%> (+100.00%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@JoseAngel1196 JoseAngel1196 marked this pull request as ready for review April 14, 2023 15:47
@JoseAngel1196 JoseAngel1196 requested a review from a team April 14, 2023 15:49
@tarkatronic
Copy link
Contributor

Give me a bit to get a handle on this... I'm actually working toward #5 right now, by way of somewhat similar tactics. Given that I'm working in a whole new reporting package though, I may be able to adopt this somewhat easily.

@JoseAngel1196
Copy link
Contributor Author

Give me a bit to get a handle on this... I'm actually working toward #5 right now, by way of somewhat similar tactics. Given that I'm working in a whole new reporting package though, I may be able to adopt this somewhat easily.

UFF perfect. Let me know! I will leave this PR open, just in case.

@JoseAngel1196 JoseAngel1196 enabled auto-merge April 14, 2023 17:49
@tarkatronic
Copy link
Contributor

@JoseAngel1196 This is showing out-of-date with main -- any chance you can update it?

@JoseAngel1196
Copy link
Contributor Author

@JoseAngel1196 This is showing out-of-date with main -- any chance you can update it?

Done! 😄


func TestSendSlackMessagesSuccess(t *testing.T) {
// Create a mock Slack client
mockClient := new(MockSlackClient)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah interesting I've never seen the new() function used. I've always opted for literals instead like mockClient := MockSlackClient{}. No need for change here, just found this interesting!

@JoseAngel1196 JoseAngel1196 merged commit 18c8725 into main Apr 18, 2023
@JoseAngel1196 JoseAngel1196 deleted the feature/slack-client-unit-test branch April 18, 2023 19:05
@tarkatronic tarkatronic added this to the Version 1.0 milestone May 16, 2023
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.

2 participants