Set rethink and mysql server DB storage tests to run on circle #824

cyli · 2016-07-11T17:05:20Z

Part of #797

This adds some basic rethinkDB TUF CRUD tests, and fixes the following issues:

MemStorage's UpdateMany previously did a partial update even if there was a conflict. Now we check for conflicts before updating.
MemStorage's UpdateMany allowed you to insert the same version twice.
RethinkDB's UpdateMany did a partial update even if there was a conflict, because the deletion criteria value (for timestamp_checksum) was passed as a list not as strings.
RethinkDB's Delete did not actually delete metadata, because the deletion criteria value
(for gun) was passed as a list and not as strings.

This PR:

factor out server MetaStore tests to take a database/store so we can have tests for every type of DB
set up tests to run on CircleCI

One or more future PRs will factor out tests for NewTUFMetaStorage to be db-testable, and do the same for the signer tests.

riyazdf · 2016-07-13T21:19:02Z

Makefile

@@ -130,16 +130,13 @@ test:
 	@echo
 	go test -tags "${NOTARY_BUILDTAGS}" $(TESTOPTS) $(PKGS)

-test-full: TESTOPTS =


Any reason for removing this target? Admittedly, I don't use it much because it doesn't include fmt and misspell

The only reason is because CI doesn't use it, and doesn't have the full linting. I can just add those, though.

(also, someone could just do make vet lint test)

yeah I'm fine with either removing it or bringing it in parity with circle. Agree that the current version isn't particularly useful

Combined all the lint functionality into one function, as per #830 (comment)

riyazdf · 2016-07-14T17:22:26Z

Makefile

-# misspell target, don't include Godeps, binaries, python tests, or git files
-misspell:
-	@echo "+ $@"
+	# mispell - requires that the following be run first:


typo: misspell

riyazdf · 2016-07-14T18:23:40Z

server/storage/memory_test.go

+	counter := make(map[string]int)
+	for _, tufObj := range expected {
+		k := entryKey(tufObj.Gun, tufObj.Role)
+		gun, ok := s.tufMeta[k]


can we rename this to gunList or versionList for clarity?

riyazdf · 2016-07-18T16:36:36Z

LGTM! Thank you for all of your work on this :)

endophage · 2016-07-19T22:06:23Z

server/storage/sqlite_test.go

@@ -0,0 +1,27 @@
+// +build !mysqldb


should this also include !rethinkdb?

Unfortunately sqldb_tests has some sample config generation that's needed by tuf_store_test, which didn't get refactored in this PR. I plan on doing that in the next PR.

That's why sqlite3 is still needed because without that refactor, the rethinkdb tests still run the tuf_store_test, which depends on stuff in sqldb_test.

endophage · 2016-07-19T22:39:35Z

Seems a little odd/confusing that the sqlite tests will not run if the mysql tests are run, but will run with the rethinkdb tests. Seems like they should either always run, or have their own build flag (rather than being !<something else>

Other than that, LGTM.

cyli · 2016-07-19T22:48:22Z

@endophage I am fixing that in the next PR - I needed to refactor some other tests in order to fix so me dependencies in sqldb_tests, so they're being run by rethink right now.

I'm planning on : no flags, sqlite3 tests run. mysqldb flag - mysql tests run. rethinkdb - rethink tests run. Does that sound right?

Signed-off-by: Ying Li <ying.li@docker.com>

…eal MySQL server too Signed-off-by: Ying Li <ying.li@docker.com>

Signed-off-by: Ying Li <ying.li@docker.com>

…ap test. Signed-off-by: Ying Li <ying.li@docker.com>

…o they can be applied to rethink. This also fixes the following bugs: 1. MemStorage's UpdateMany previously did a partial update even if there was a conflict. Now we check for conflicts before updating. 2. MemStorage's UpdateMany allowed you to insert the same version twice. 3. RethinkDB's UpdateMany does a partial update even if there was a conflict, because the deletion criteria value (for timestamp_checksum) was passed as a list not as strings. 4. RethinkDB's delete did not actually delete metadata, because the deletion criteria value (for gun) was passed as a list and not as strings. Signed-off-by: Ying Li <ying.li@docker.com>

…ion test are separate. Signed-off-by: Ying Li <ying.li@docker.com>

Signed-off-by: Ying Li <ying.li@docker.com>

…d old metadata is no longer current. Also rename a variable to be more clear, and make invalid arguments to the integration/testdb scripts fail faster. Signed-off-by: Ying Li <ying.li@docker.com>

endophage · 2016-07-19T22:54:42Z

@cyli sounds perfect! Merging ~~now~~ when CI finishes

GordonTheTurtle added status/0-triage labels Jul 11, 2016

cyli force-pushed the db-tests-refactor branch 8 times, most recently from e2e85d3 to ffdc5d2 Compare July 12, 2016 08:01

cyli changed the title ~~WIP setting up rethink and mysql tests to run on circle~~ WIP Set rethink and mysql server DB storage tests to run on circle Jul 12, 2016

cyli force-pushed the db-tests-refactor branch 2 times, most recently from 120869a to c801293 Compare July 12, 2016 23:05

cyli changed the title ~~WIP Set rethink and mysql server DB storage tests to run on circle~~ Set rethink and mysql server DB storage tests to run on circle Jul 13, 2016

cyli changed the title ~~Set rethink and mysql server DB storage tests to run on circle~~ WIP: Set rethink and mysql server DB storage tests to run on circle Jul 13, 2016

cyli force-pushed the db-tests-refactor branch 3 times, most recently from b47ce44 to a34a946 Compare July 13, 2016 21:15

cyli changed the title ~~WIP: Set rethink and mysql server DB storage tests to run on circle~~ Set rethink and mysql server DB storage tests to run on circle Jul 13, 2016

cyli force-pushed the db-tests-refactor branch from a34a946 to 4f5435c Compare July 13, 2016 21:18

riyazdf reviewed Jul 13, 2016
View reviewed changes

cyli force-pushed the db-tests-refactor branch from 4f5435c to baf8c1a Compare July 14, 2016 17:02

riyazdf reviewed Jul 14, 2016
View reviewed changes

cyli force-pushed the db-tests-refactor branch 2 times, most recently from ea97efd to f9caac5 Compare July 14, 2016 18:11

riyazdf mentioned this pull request Jul 14, 2016

Wait for indexes in rethink table.wait #834

Merged

riyazdf reviewed Jul 14, 2016
View reviewed changes

cyli force-pushed the db-tests-refactor branch 2 times, most recently from 985502d to 52a51ac Compare July 14, 2016 23:01

cyli force-pushed the db-tests-refactor branch from 52a51ac to 2ee3410 Compare July 19, 2016 18:03

endophage reviewed Jul 19, 2016
View reviewed changes

cyli added 8 commits July 19, 2016 15:49

Rename server and signer storage items to reflect the db type

ddb2860

Signed-off-by: Ying Li <ying.li@docker.com>

Split SQL DB creation into a separate file so we can test against a r…

4f43dab

…eal MySQL server too Signed-off-by: Ying Li <ying.li@docker.com>

Set up mysql server tests to run on circle

5feac09

Signed-off-by: Ying Li <ying.li@docker.com>

Enable tests that require a rethink connection too, and add a bootstr…

c61be06

…ap test. Signed-off-by: Ying Li <ying.li@docker.com>

Fix lint issue, and reconfigure build scripts so db test and integrat…

b76b1cb

…ion test are separate. Signed-off-by: Ying Li <ying.li@docker.com>

Fix comment and combine linting make targets into a single one.

2341a2a

Signed-off-by: Ying Li <ying.li@docker.com>

Fix a somewhat buggy testUpdateManyNoConflicts (as metadata is update…

1eb6094

…d old metadata is no longer current. Also rename a variable to be more clear, and make invalid arguments to the integration/testdb scripts fail faster. Signed-off-by: Ying Li <ying.li@docker.com>

cyli force-pushed the db-tests-refactor branch from 2ee3410 to 1eb6094 Compare July 19, 2016 22:49

endophage merged commit 1023a94 into notaryproject:master Jul 19, 2016

cyli deleted the db-tests-refactor branch July 19, 2016 23:29

cyli mentioned this pull request Jul 20, 2016

More server DB tests #854

Merged

This was referenced Aug 2, 2016

Refactor server/signer tests to be able to test multiple DBs (like rethink) #797

Closed

atomically updates #527

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set rethink and mysql server DB storage tests to run on circle #824

Set rethink and mysql server DB storage tests to run on circle #824

cyli commented Jul 11, 2016 •

edited

Loading

riyazdf Jul 13, 2016

cyli Jul 13, 2016

cyli Jul 13, 2016

riyazdf Jul 13, 2016

cyli Jul 14, 2016

riyazdf Jul 14, 2016

riyazdf Jul 14, 2016

riyazdf commented Jul 18, 2016

endophage Jul 19, 2016

cyli Jul 19, 2016

endophage commented Jul 19, 2016

cyli commented Jul 19, 2016

endophage commented Jul 19, 2016 •

edited

Loading

Set rethink and mysql server DB storage tests to run on circle #824

Set rethink and mysql server DB storage tests to run on circle #824

Conversation

cyli commented Jul 11, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

riyazdf commented Jul 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

endophage commented Jul 19, 2016

cyli commented Jul 19, 2016

endophage commented Jul 19, 2016 • edited Loading

cyli commented Jul 11, 2016 •

edited

Loading

endophage commented Jul 19, 2016 •

edited

Loading