Fixing SQL #338

Temikus · 2018-05-22T00:25:46Z

A lot of SQL refactoring, mostly tests. This will slow tests down a bit but will fix 99% of the flakiness, and add proper lifecycle tests.

SQL Tests

Add new test factories for SQLv1/v2 and SQL Users/SslCerts and instantiated basic collection tests
Major refactor of SQLv1 and SQLv2 tests, getting rid of old shared helpers and tests that were leaving cruft in test projects and were very flaky, incl.:
- Adding SQL users factory and migrate user tests to it
- Adding SslCert factory
- Breaking out tests by subject collection

Minor CI/Test improvements

Make monitoring cleanup less chatty
Bump GoogleSQL instance tier to one with SLA and MakeSQL tests serial
Set github status if any CI task failed
Tweak rubocop rules
Add task to lint changed files

Drive-by improvements:

Set SQL delete/create operations to synchronous
Fixes for SQL Users model workflow
- If Etag is nil then User doesn't exist, so should be inserted,
  not the other way around.
- Changed async to positional parameter to match other models.
Minor refactor of tier tests
- Remove list test as .all depends on list
- Scope down the test to not instantiate a full client
Improve disk type test to include more regions

Fixes #337

houndci-bot · 2018-05-22T00:40:30Z

test/integration/sql/test_v1_instances.rb

+    @factory = SqlV1Factory.new(namespaced_name)
+  end
+
+end


Layout/TrailingBlankLines: Final newline missing.

houndci-bot · 2018-05-22T00:40:30Z

test/integration/sql/test_v1_instances.rb

+    @subject = Fog::Google[:sql].instances
+    @factory = SqlV1Factory.new(namespaced_name)
+  end
+


Layout/EmptyLinesAroundClassBody: Extra empty line detected at class body end.

houndci-bot · 2018-05-22T01:07:35Z

test/integration/sql/test_v1_users.rb

+    @subject = Fog::Google[:sql].instances
+    @factory = SqlV1InstancesFactory.new(namespaced_name)
+  end
+end


Layout/TrailingBlankLines: Final newline missing.

houndci-bot · 2018-05-22T01:07:35Z

test/integration/sql/test_v1_instances.rb

+    assert_equal(labels, updated.settings[:user_labels])
+    assert_operator(updated.settings_version, :>, settings_version)
+  end
+end


Layout/TrailingBlankLines: Final newline missing.

houndci-bot · 2018-05-22T01:07:35Z

test/integration/sql/test_v1_instances.rb

+
+    settings_version = instance.settings_version
+    labels = {
+        :foo => "bar"


Layout/IndentHash: Use 2 spaces for indentation in a hash, relative to the start of the line where the left curly brace is.

houndci-bot · 2018-05-22T02:26:36Z

test/integration/sql/test_v1_users.rb

+        "expected no user #{user.name}"
+    )
+  end
+end


Layout/TrailingBlankLines: Final newline missing.

houndci-bot · 2018-05-22T02:26:36Z

test/integration/sql/test_v1_users.rb

+    # Delete user
+    users.first.destroy(:async => false)
+    assert_empty(
+        @subject.all(user.instance).select { |u| u.name == user.name },


Layout/FirstParameterIndentation: Indent the first parameter one step more than the start of the previous line.

houndci-bot · 2018-05-22T02:26:36Z

test/integration/factories/sql_v1_users_factory.rb

+
+  def cleanup
+     # Users will be cleaned up with the test instance.
+     @instances.cleanup


Layout/IndentationWidth: Use 2 (not 3) spaces for indentation.

houndci-bot · 2018-05-22T04:26:03Z

test/integration/sql/test_v1_certs.rb

+    @subject.get(instance_name, ssl_cert2.sha1_fingerprint).tap do |result|
+        assert_equal(ssl_cert2.common_name, result.common_name)
+        assert_equal("sql#sslCert", result.kind)
+      end


Layout/BlockAlignment: end at 33, 6 is not aligned with @subject.get(instance_name, ssl_cert2.sha1_fingerprint).tap do |result| at 30, 4.

houndci-bot · 2018-05-22T04:26:04Z

test/integration/sql/test_v1_certs.rb

+    ssl_cert2 = @subject.new(
+                     :common_name => "#{ssl_cert.common_name}-2" ,
+                     :instance => instance_name
+                     )


Layout/ClosingParenthesisIndentation: Indent ) the same as the start of the line where ( is.

houndci-bot · 2018-05-22T04:26:04Z

test/integration/sql/test_v1_certs.rb

+
+    # Create a second cert and attach to the same instance
+    ssl_cert2 = @subject.new(
+                     :common_name => "#{ssl_cert.common_name}-2" ,


Layout/FirstParameterIndentation: Indent the first parameter one step more than the start of the previous line.
Layout/SpaceBeforeComma: Space found before comma.

houndci-bot · 2018-05-22T04:26:04Z

test/integration/factories/sql_v1_certs_factory.rb

+
+  def cleanup
+     # Certs will be cleaned up with the test instance.
+     @instances.cleanup


Layout/IndentationWidth: Use 2 (not 3) spaces for indentation.

houndci-bot · 2018-05-22T09:00:36Z

test/integration/sql/test_v1_certs.rb

+
+    # Create a second cert and attach to the same instance
+    ssl_cert2 = @subject.new( :common_name => "#{ssl_cert.common_name}-2",
+                              :instance => instance_name )


Layout/SpaceInsideParens: Space inside parentheses detected.

houndci-bot · 2018-05-22T09:00:36Z

test/integration/sql/test_v1_certs.rb

+    instance_name = ssl_cert.instance
+
+    # Create a second cert and attach to the same instance
+    ssl_cert2 = @subject.new( :common_name => "#{ssl_cert.common_name}-2",


Layout/SpaceInsideParens: Space inside parentheses detected.

houndci-bot · 2018-05-22T10:58:12Z

test/integration/sql/test_v2_instances.rb

+    description = "test backup run"
+    operation = wait_until_complete do
+      @client.insert_backup_run(
+          instance.name,


Layout/FirstParameterIndentation: Indent the first parameter one step more than the start of the previous line.

houndci-bot · 2018-05-24T04:25:48Z

tasks/test.rake

    t.libs << "test"
-    t.pattern = FileList['test/integration/sql/test_*.rb']
+    t.pattern = FileList['test/integration/sql/test_v2*.rb']


Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-05-24T04:25:48Z

tasks/test.rake

+    t.name = "sqlv1"
+    t.description = "Run SQLv1 API tests"
+    t.libs << "test"
+    t.pattern = FileList['test/integration/sql/test_common*.rb','test/integration/sql/test_v1*.rb']


Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Layout/SpaceAfterComma: Space missing after comma.

houndci-bot · 2018-05-24T04:25:48Z

tasks/test.rake

+  multitask :sql_parallel => [:sqlv1, :sqlv2]
+
+  desc 'Run all SQL API tests'
+  task :sql => [:sqlv1, :sqlv2]


Style/SymbolArray: Use %i or %I for an array of symbols.

houndci-bot · 2018-05-24T04:25:48Z

tasks/test.rake

+  desc 'Run all SQL API tests in parallel'
+  multitask :sql_parallel => [:sqlv1, :sqlv2]
+
+  desc 'Run all SQL API tests'


Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-05-24T04:25:48Z

tasks/test.rake

@@ -46,11 +47,26 @@ namespace :test do
    t.verbose = true
  end

+  desc 'Run all SQL API tests in parallel'
+  multitask :sql_parallel => [:sqlv1, :sqlv2]


Style/SymbolArray: Use %i or %I for an array of symbols.

houndci-bot · 2018-05-24T04:25:48Z

tasks/test.rake

@@ -46,11 +47,26 @@ namespace :test do
    t.verbose = true
  end

+  desc 'Run all SQL API tests in parallel'


Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-05-24T04:25:49Z

tasks/test.rake

    t.libs << "test"
-    t.pattern = FileList['test/integration/sql/test_*.rb']
+    t.pattern = FileList['test/integration/sql/test_v2*.rb']


Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-05-24T04:25:49Z

tasks/test.rake

+    t.name = "sqlv1"
+    t.description = "Run SQLv1 API tests"
+    t.libs << "test"
+    t.pattern = FileList['test/integration/sql/test_common*.rb','test/integration/sql/test_v1*.rb']


Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Layout/SpaceAfterComma: Space missing after comma.

houndci-bot · 2018-05-24T04:25:49Z

tasks/test.rake

+  multitask :sql_parallel => [:sqlv1, :sqlv2]
+
+  desc 'Run all SQL API tests'
+  task :sql => [:sqlv1, :sqlv2]


Style/SymbolArray: Use %i or %I for an array of symbols.

houndci-bot · 2018-05-24T04:25:49Z

tasks/test.rake

+  desc 'Run all SQL API tests in parallel'
+  multitask :sql_parallel => [:sqlv1, :sqlv2]
+
+  desc 'Run all SQL API tests'


Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-05-24T04:25:49Z

tasks/test.rake

@@ -46,11 +47,26 @@ namespace :test do
    t.verbose = true
  end

+  desc 'Run all SQL API tests in parallel'
+  multitask :sql_parallel => [:sqlv1, :sqlv2]


Style/SymbolArray: Use %i or %I for an array of symbols.

houndci-bot · 2018-05-24T04:25:50Z

tasks/test.rake

@@ -46,11 +47,26 @@ namespace :test do
    t.verbose = true
  end

+  desc 'Run all SQL API tests in parallel'


Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

Temikus · 2018-05-24T07:14:11Z

SQL Api is being flaky again, it seems -_-

I'll try to figure this out, probably will need to wrap with Retriable like with Stackdriver...

houndci-bot · 2018-05-24T07:16:34Z

tasks/test.rake

+    t.name = "sqlv1"
+    t.description = "Run SQLv1 API tests"
+    t.libs << "test"
+    t.pattern = FileList["test/integration/sql/test_common*.rb","test/integration/sql/test_v1*.rb"]


Layout/SpaceAfterComma: Space missing after comma.

houndci-bot · 2018-05-24T07:16:34Z

tasks/test.rake

+  multitask :sql_parallel => [:sqlv1, :sqlv2]
+
+  desc "Run all SQL API tests"
+  task :sql => [:sqlv1, :sqlv2]


Style/SymbolArray: Use %i or %I for an array of symbols.

houndci-bot · 2018-05-24T07:16:34Z

tasks/test.rake

+  end
+
+  desc "Run all SQL API tests in parallel"
+  multitask :sql_parallel => [:sqlv1, :sqlv2]


Style/SymbolArray: Use %i or %I for an array of symbols.

Temikus · 2018-05-24T07:22:44Z

Ok, it seems only us-central1 is misbehaving. Australia is fine. I'll try to choose another US region first but if this is persistent I'll need to investigate the backend.

Temikus · 2018-05-24T11:17:47Z

So I get tons of Google::Apis::ClientError: invalidState: The instance or operation is not in an appropriate state to handle the request. even though the operations reported as ready. I've synchronised almost everything. All of those errors are in cleanup BUT it still cleans those up properly 0_o. Probably those resources were not designed to be created/destroyed in 10 minutes.

I'll need to sleep on this... Maybe add a simple wait to teardown or something.

houndci-bot · 2018-06-23T01:44:17Z

tasks/test.rake

    t.warning = false
    t.verbose = true
  end
+


Layout/TrailingWhitespace: Trailing whitespace detected.

- Changes to instantiate proper collection tests: -- Add sql users factory and migrate user tests to it -- Adding SslCert factory -- Breaking out tests by subject collection - Removing old v1 Tests - Drive-by fix for SQL Users model workflow -- If Etag is nil then User doesn't exist, so should be inserted, not the other way around. -- Changed async to positional parameter to match other models.

- Remove list test as .all depends on list - Scope down the test to not instantiate a full client

Refactored old SQLv2 tests, deleting old shared

To locally preempt HoundCI where needed

- Make monitoring cleanup less chatty - Bump GoogleSQL instance tier - Since Shared CPU machine types (db-f1-micro and db-g1-small) are not covered by the Cloud SQL SLA. See: https://cloud.google.com/sql/pricing - Set github status if any CI task failed - Make SQL tests serial - Tweak rubocop rules

Temikus · 2018-06-23T02:10:52Z

This is now finalised, waiting for integration tests and we should be good to go.

houndci-bot reviewed May 22, 2018

View reviewed changes

Temikus force-pushed the fix_sql branch 2 times, most recently from 0b53fdf to 4e1a115 Compare May 22, 2018 02:26

houndci-bot reviewed May 22, 2018

View reviewed changes

Temikus added the integrate label May 22, 2018

Temikus force-pushed the fix_sql branch 2 times, most recently from 5c0a1ef to 826d293 Compare May 24, 2018 04:05

houndci-bot reviewed May 24, 2018

View reviewed changes

Temikus force-pushed the fix_sql branch from 9b007bc to 99e9f31 Compare May 24, 2018 04:27

Temikus force-pushed the fix_sql branch from f6920c6 to 1239fce Compare May 24, 2018 07:16

houndci-bot reviewed May 24, 2018

View reviewed changes

Temikus force-pushed the fix_sql branch from b1e87be to 61407cf Compare May 24, 2018 09:57

Temikus force-pushed the fix_sql branch 2 times, most recently from 00ee296 to 22381bb Compare June 5, 2018 21:43

Temikus force-pushed the fix_sql branch 2 times, most recently from 6040ca3 to c2a2f9e Compare June 23, 2018 01:44

houndci-bot reviewed Jun 23, 2018

View reviewed changes

tasks/test.rake Outdated

t.warning = false

t.verbose = true

end

Copy link

houndci-bot Jun 23, 2018

Choose a reason for hiding this comment

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

Layout/TrailingWhitespace: Trailing whitespace detected.

Temikus added 5 commits June 23, 2018 11:46

Set SQL delete/create operations to synchronous

c5ed381

Improve disk type test

a5b89a9

Minor refactor of tier tests

416c7ab

- Remove list test as .all depends on list - Scope down the test to not instantiate a full client

Refactor SQLv2 tests

b91c767

Refactored old SQLv2 tests, deleting old shared

Temikus and others added 5 commits June 23, 2018 11:46

Update README's

22b1d5e

Add task to lint changed files

90de67b

To locally preempt HoundCI where needed

Add backwards compatibility

ea4be8b

Add to CHANGELOG.md

5af8e9f

Temikus force-pushed the fix_sql branch from c2a2f9e to 5af8e9f Compare June 23, 2018 02:10

Temikus changed the title ~~WIP - Fixing SQL~~ Fixing SQL Jun 23, 2018

Temikus added integrate and removed integrate labels Jun 23, 2018

Temikus merged commit 1185ec1 into fog:master Jun 23, 2018

Temikus deleted the fix_sql branch September 28, 2019 07:07

Fixing SQL #338

Fixing SQL #338

Conversation

Temikus commented May 22, 2018 • edited Loading

SQL Tests

Minor CI/Test improvements

Drive-by improvements:

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

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

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

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

Temikus commented May 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Temikus commented May 24, 2018

Temikus commented May 24, 2018

Choose a reason for hiding this comment

Temikus commented Jun 23, 2018

Temikus commented May 22, 2018 •

edited

Loading