-
Notifications
You must be signed in to change notification settings - Fork 42
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
Unit tests #1062
Unit tests #1062
Conversation
Your Testserver will be ready at https://1062.test.live.mm.rbg.tum.de in a few minutes. Logins
|
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.
Great, thanks! Just a quick hint
api/progress.go
Outdated
@@ -176,8 +177,16 @@ func (r progressRoutes) markWatched(c *gin.Context) { | |||
} | |||
|
|||
func (r progressRoutes) getProgressBatch(c *gin.Context) { | |||
tumLiveContext := c.MustGet("TUMLiveContext").(tools.TUMLiveContext) | |||
ctx, exists := c.Get("TUMLiveContext") |
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.
Was there an issue with the MustGet
approach?
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.
For the progress API we don't use a middleware as tools.InitCourse(daoWrapper)
and therefore the handler panics every time the context doesn't exists instead of returning 400
.
* Add notification tests * Add progress tests * Add course tests * Add more course tests * Remove println * Fix context error
Motivation and Context
In the last few months we've added a lot of new functionality without implementing associated unit tests. This PR addresses this issue partly (Not all functions can be tested).
Description
Add unit tests.
Steps for Testing
go test / test
workflow on GitHub succeeds