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

Unit tests #1062

Merged
merged 6 commits into from
Jun 21, 2023
Merged

Unit tests #1062

merged 6 commits into from
Jun 21, 2023

Conversation

MatthiasReumann
Copy link
Collaborator

@MatthiasReumann MatthiasReumann commented Jun 12, 2023

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

@github-actions
Copy link

Your Testserver will be ready at https://1062.test.live.mm.rbg.tum.de in a few minutes.

Logins
Kurs1 Kurs2 Kurs3 Kurs4
public public loggedin enrolled
prof1 prof1 prof2 prof1
prof2
student1
student2
student3
student1
student2
student2
student3
student1
student2

@MatthiasReumann MatthiasReumann marked this pull request as ready for review June 12, 2023 11:28
Copy link
Sponsor Member

@joschahenningsen joschahenningsen left a 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")
Copy link
Sponsor Member

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?

Copy link
Collaborator Author

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.

api/progress.go Outdated Show resolved Hide resolved
api/progress.go Outdated Show resolved Hide resolved
@joschahenningsen joschahenningsen merged commit 9138428 into dev Jun 21, 2023
@joschahenningsen joschahenningsen deleted the enh/tests-code-coverage branch June 21, 2023 11:16
SebiWrn pushed a commit that referenced this pull request May 7, 2024
* Add notification tests

* Add progress tests

* Add course tests

* Add more course tests

* Remove println

* Fix context error
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