Skip to content
This repository has been archived by the owner on Aug 16, 2022. It is now read-only.

test: add Mongo Asset's FindByID unit testing #139

Merged
merged 8 commits into from
May 11, 2022

Conversation

KeisukeYamashita
Copy link
Contributor

@KeisukeYamashita KeisukeYamashita commented May 8, 2022

Overview

I have added initial unit testing for the MongoDB based on this issue for "small start" 👉 reearth/reearth-visualizer#273.
See details (requirements) in that issue.

I implemented one for a trial to avoid conflicts and decide how we are going to test this methods.

What I've done

I have added unit test for the Asset repository's FindByID method.

What I haven't done

Add unit tests for all methods for Assets or other repositories.
This is because I wanted to "start small" since we need to discuss the policy and the implementation design first so that we don't waste time.

How I tested

The testings are done by the CI GitHub Actions workflow.
This can be also tested locally.

# Skip testing
go test -timeout 30s -run '^TestFindByID$' github.com/reearth/reearth-backend/internal/infrastructure/mongo

# Test (after launching MongoDB instance in your local environment)
REEARTH_DB=mongodb://localhost go test -timeout 30s -run '^TestFindByID$' github.com/reearth/reearth-backend/internal/infrastructure/mongo

Which point I want you to review particularly

  • How the testing(s) should be done
    • Table testing
    • Variable namings
    • etc

Memo

Please feel free to give me any feedback as this is the starting point of adding unit tests to the repository layer.

@KeisukeYamashita KeisukeYamashita self-assigned this May 8, 2022
@codecov
Copy link

codecov bot commented May 8, 2022

Codecov Report

Merging #139 (bc0019a) into main (4f72b87) will decrease coverage by 2.06%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #139      +/-   ##
==========================================
- Coverage   36.68%   34.61%   -2.07%     
==========================================
  Files         331      349      +18     
  Lines       29890    31755    +1865     
==========================================
+ Hits        10964    10992      +28     
- Misses      17925    19760    +1835     
- Partials     1001     1003       +2     
Impacted Files Coverage Δ
internal/infrastructure/mongo/config.go 0.00% <0.00%> (ø)
internal/infrastructure/mongo/dataset_schema.go 0.00% <0.00%> (ø)
internal/infrastructure/mongo/property_schema.go 0.00% <0.00%> (ø)
internal/infrastructure/mongo/plugin.go 0.00% <0.00%> (ø)
internal/infrastructure/mongo/tag.go 0.00% <0.00%> (ø)
internal/infrastructure/mongo/dataset.go 0.00% <0.00%> (ø)
internal/infrastructure/mongo/asset.go 25.26% <0.00%> (ø)
internal/infrastructure/mongo/auth_request.go 0.00% <0.00%> (ø)
internal/infrastructure/mongo/scene_lock.go 0.00% <0.00%> (ø)
internal/infrastructure/mongo/team.go 0.00% <0.00%> (ø)
... and 8 more

Signed-off-by: KeisukeYamashita <[email protected]>
internal/infrastructure/mongo/asset_test.go Outdated Show resolved Hide resolved
internal/infrastructure/mongo/asset_test.go Outdated Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
@KeisukeYamashita KeisukeYamashita changed the title [WIP] Add Mongo Asset's FindByID unit testing Add Mongo Asset's FindByID unit testing May 8, 2022
@KeisukeYamashita KeisukeYamashita marked this pull request as ready for review May 8, 2022 04:21
@KeisukeYamashita KeisukeYamashita requested a review from rot1024 as a code owner May 8, 2022 04:21
@KeisukeYamashita
Copy link
Contributor Author

KeisukeYamashita commented May 8, 2022

TODO ✔️

  • Need to add the REEARTH_DB env for testing in the CI

@rot1024 rot1024 changed the title Add Mongo Asset's FindByID unit testing test: add Mongo Asset's FindByID unit testing May 9, 2022
Copy link
Member

@rot1024 rot1024 left a comment

Choose a reason for hiding this comment

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

Thank you! How about creating a helper function like this one?

func connect(t *testing.T) func() (*mongodoc.Client, func()) {
	t.Helper()

	// Skip unit testing if "REEARTH_DB" is not configured
	// See details: https://github.com/reearth/reearth/issues/273
	db := os.Getenv("REEARTH_DB")
	if db == "" {
		t.SkipNow()
		return nil
	}

	c, _ := mongo.Connect(
		context.Background(),
		options.Client().
			ApplyURI(db).
			SetConnectTimeout(time.Second*10),
	)

	return func() {
		database, _ := uuid.New()
		databaseName := "reearth-test-" + string(database[:])
		client := mongodoc.NewClient(databaseName, c)
		return client, func() {
			_ = c.Database(databaseName).Drop(context.Background())
		}
	}
}

Usage:

func TestFindByID(t *testing.T) {
	// ...
	initDB := connect(t)
 
	for _, tc := range tests {
		tc := tc
		t.Run(tc.Name, func() {
			t.Paralell()
			client, dropDB := initDB()
			defer dropDB()

			repo := NewAsset(client)	  
			ctx := context.Background()
			err := repo.Save(ctx, want)
			assert.NoError(t, err)
	  
			got, err := repo.FindByID(ctx, want.ID())
			// ...
		})
	}
}

internal/infrastructure/mongo/asset_test.go Outdated Show resolved Hide resolved
@KeisukeYamashita KeisukeYamashita force-pushed the test-mongo-asset-findbyid branch from 06278f8 to 7810ec8 Compare May 9, 2022 14:07
@KeisukeYamashita KeisukeYamashita marked this pull request as draft May 9, 2022 15:02
@KeisukeYamashita KeisukeYamashita marked this pull request as ready for review May 9, 2022 15:50
@KeisukeYamashita KeisukeYamashita requested a review from rot1024 May 9, 2022 15:50
@KeisukeYamashita
Copy link
Contributor Author

Thank you for your review 😄

@rot1024 rot1024 merged commit 35f9db5 into main May 11, 2022
@rot1024 rot1024 deleted the test-mongo-asset-findbyid branch May 11, 2022 02:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants