Skip to content

Conversation

@Vafilor
Copy link
Contributor

@Vafilor Vafilor commented Jun 25, 2020

What this PR does:

Sets up unit testing with a live database.

Fixes onepanelio/core#

Special notes for your reviewer:

run make test-run to run the tests.

Vafilor added 30 commits June 25, 2020 16:27
… sql and go migrations. This makes it easier to unit test where we just need sql migrations.
…ts to k8s mock will have them be automatically loaded.
… sql and go migrations. This makes it easier to unit test where we just need sql migrations.
…ts to k8s mock will have them be automatically loaded.
updated code to reflect this change and migrated it over.
@rushtehrani rushtehrani added this to the v0.12.0 milestone Jul 1, 2020
}

func Test_WorkspaceStatusToFieldMap(t *testing.T) {
testWorkspaceStatusToFieldMapLaunching(t)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you elaborate on the prefixes in test function names? I see:

test
Test
Test_
testClient (No _ here as opposed to the public funcs)
TestClient_

I am assuming TestClient_ are tests for methods on instances of DefaultTestClient(), and methods prefixed with testClient are for the private methods. But then looking at this line, that doesn't seem to be the case? Also should we consistently use _ for both public and private methods?

Copy link
Contributor Author

@Vafilor Vafilor Jul 9, 2020

Choose a reason for hiding this comment

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

General form is Test[<Struct>]_<Method> where <struct> is optional. This format comes from Goland - if you type in func Test.... it'll provide auto complete options to this.

TestClient_ are for methods on Client struct, which is a majority of the methods in pkg.
So we have TestClient_<method_name>. Methods like these are for direct tests of a method, they should completely test a method.

To test specific cases, you have testClient methods. Format is testClient<MethodName><Condition>
e.g. testClientGetWorkflowTemplateDBEmpty tests the GetWorkflowTemplateDB method, when the there is no template - empty.

TestClientGetWorkflowTemplateDB would call this as a child method.

TestClientGetWorfklowTemplateDB(...) {
   testClientGetWorkflowTemplateDBEmpty(...)
}

In the case where we have two methods where the only name difference is case, like
GetWorkflowTemplate vs getWorkflowTemplate I've used the private word.

testClientPrivateGetWorkflowTemplate
vs
testClientGetWorkflowTemplate which is for the exported member.

@rushtehrani rushtehrani changed the base branch from dev to feat/provider-specific-config July 9, 2020 18:28
@rushtehrani rushtehrani changed the base branch from feat/provider-specific-config to dev July 9, 2020 18:29
`
)

func TestParseWorkspaceSpec(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be updated to match the new convention?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'll update it.

@rushtehrani
Copy link
Contributor

Getting this error when running this branch:

Exiting.

I have already ran this migration so I'm not sure why it is trying it again.

@Vafilor
Copy link
Contributor Author

Vafilor commented Jul 9, 2020

Getting this error when running this branch:

Exiting.

I have already ran this migration so I'm not sure why it is trying it again.

Are you running it in test mode, or just general build mode?

@rushtehrani
Copy link
Contributor

Are you running it in test mode, or just general build mode?

Running in debug mode - not testing.

Vafilor added 2 commits July 9, 2020 12:23
…em if they were ran earlier in the goose_db_version db table.
… printed after the goose message, which was a bit confusing.
@rushtehrani
Copy link
Contributor

When a test fails, the test container is not stopped and I get this error when trying to run tests:

 $ make test-run
make test-start
docker run --rm --name test-onepanel-postgres -p 5432:5432 -e POSTGRES_USER=admin -e POSTGRES_PASSWORD=tester -e POSTGRES_DB=onepanel -d  postgres:12.3
docker: Error response from daemon: Conflict. The container name "/test-onepanel-postgres" is already in use by container "f30a3914dcdf324348b13ff48bfe15cc41a7fde5c551a95d48c33e5234b2f4ce". You have to remove (or rename) that container to be able to reuse that name.
See 'docker run --help'.
make[1]: *** [test-start] Error 125
make: *** [test-run] Error 2

@rushtehrani rushtehrani merged commit 46f4465 into dev Jul 9, 2020
@rushtehrani rushtehrani deleted the test/docker.database branch July 9, 2020 21:17
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.

3 participants