-
Notifications
You must be signed in to change notification settings - Fork 11
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
AWS storage implementation tests #321
Conversation
f853ecd
to
902bcba
Compare
storage/aws/aws_test.go
Outdated
"github.com/transparency-dev/trillian-tessera/api" | ||
"github.com/transparency-dev/trillian-tessera/api/layout" | ||
storage "github.com/transparency-dev/trillian-tessera/storage/internal" | ||
"k8s.io/klog" |
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.
Might want k8s.io/klog/v2
to match Tessera?
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.
and the aws file... thanks.
storage/aws/aws_test.go
Outdated
// TestMain parses flags. | ||
func TestMain(m *testing.M) { | ||
klog.InitFlags(nil) | ||
flag.Parse() |
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.
You may not need the call to flag.Parse
here as M.Run()
will parse flags if they've not already been parsed: https://cs.opensource.google/go/go/+/refs/tags/go1.23.3:src/testing/testing.go;l=1942-1945
storage/aws/aws_test.go
Outdated
os.Exit(m.Run()) | ||
} | ||
|
||
// canSkipMySQLTest checks if the test MySQL db is available and if not, if the test can be skipped. |
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.
// canSkipMySQLTest checks if the test MySQL db is available and if not, if the test can be skipped. | |
// canSkipMySQLTest checks if the test MySQL db is available and, if not, the test can be skipped. |
storage/aws/aws_test.go
Outdated
|
||
// canSkipMySQLTest checks if the test MySQL db is available and if not, if the test can be skipped. | ||
// | ||
// Use this method before every MySQL test, and if it return true, exist the test. |
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.
// Use this method before every MySQL test, and if it return true, exist the test. | |
// Use this method before every MySQL test, and if it returns true, exit the test. |
storage/aws/aws_test.go
Outdated
klog.Warningf("MySQL not available, skipping %s", t.Name()) | ||
return |
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.
Could you use t.Skip("MySQL not available)
here (& elsewhere)?
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.
Ah there is it! I was looking for t.Return.. Thanks, I've used this, and left the warning messages. Otherwise one needs to specify -v to realize that these tests are not running.
902bcba
to
e818918
Compare
Towards #312 and #24.
This PR builds upon the GCP tests, and migrates them to MySQL and S3.
This file contains some MySQL and some non-MySQL tests. MySQL tests should only run if a MySQL database is available, inspired by the
storage/mysq/mysql_test.go
setup. I tried a bunch of setups with "TestMain" and "t.Clean", and settled on this setup with two helper methods, and disabling parallel tests.This PR has two commits: one that moves tests, and one that migrates them to AWS to ease reviewing (separating them into two PRs is challenging because of package names and private functions).