You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2020/08/28 11:29:18 UTC

[GitHub] [couchdb] dottorblaster opened a new pull request #3110: Tag elixir tests in meaningful groups

dottorblaster opened a new pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110


   
   ## Overview
   
   This is a first attempt to assign meaningful categories to tests into our Elixir integration test suite. For now I only modified the `moduletag` attribute on the single modules, once I have some positive feedback on that I'll proceed tagging single test cases.
   
   ## Testing recommendations
   
   Just issue `make elixir` or wait for the CI
   
   ## Related Issues or Pull Requests
   
   #1885 
   
   ## Checklist
   
   - [X] Code is written and works correctly
   - [X] Changes are covered by tests
   - [X] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
   - [X] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] dottorblaster commented on pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
dottorblaster commented on pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#issuecomment-683468123


   @iilyak I changed my mind about moduletags even if I'd prefer a moduletag in every test module. Going forth, I tagged every test with its corresponding setup, based on my understanding of the test suite and of CouchDB. Could you review this?
   
   Once this is ready I think we can start experimenting with splitting tests execution inside Makefile, that can be done with (e.g.) `mix test --trace --only kind:single_node $(EXUNIT_OPTS)`


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on a change in pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#discussion_r480324346



##########
File path: test/elixir/test/partition_view_update_test.exs
##########
@@ -5,6 +5,10 @@ defmodule PartitionViewUpdateTest do
   @moduledoc """
   Test Partition view update functionality
   """
+  
+  @moduletag :partition
+  @moduletag kind: :cluster

Review comment:
       This should be :single_node. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
iilyak commented on pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#issuecomment-683764902


   I hope we didn't have tests which make sense to run using multiple setups.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] dottorblaster edited a comment on pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
dottorblaster edited a comment on pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#issuecomment-683437841


   As one great dev once said, "maybe we should hire a poet to name things" - cfr. @riquito
   
   Be it `setup`, `kind` or `type`, I grokked the concept. Thanks, I'm gonna try it soon


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] dottorblaster commented on pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
dottorblaster commented on pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#issuecomment-683912481


   @iilyak hahah I have to say it was a bit frustrating but also one of the funniest things happened to me on GitHub, thanks for the fun :-D
   
   Anyway, I fixed everything tagging modules instead of single tests, adopting the same tags I used in the previous approach. I think it might be good, please review again and sorry if I got you wrong!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on a change in pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#discussion_r480126535



##########
File path: test/elixir/test/changes_async_test.exs
##########
@@ -7,18 +7,21 @@ defmodule ChangesAsyncTest do
   Test CouchDB /{db}/_changes
   """
 
+  @tag kind: :performance

Review comment:
       I think all tests in this module should be `:single_node`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on a change in pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#discussion_r480323820



##########
File path: test/elixir/test/partition_ddoc_test.exs
##########
@@ -4,6 +4,9 @@ defmodule PartitionDDocTest do
   @moduledoc """
   Test partition design doc interactions
   """
+  
+  @moduletag :partition
+  @moduletag kind: :cluster

Review comment:
       This should be :single_node. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] dottorblaster edited a comment on pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
dottorblaster edited a comment on pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#issuecomment-683437841


   As one great dev once said, "maybe we should hire a poet to name things" - cit. @riquito
   
   Be it `setup`, `kind` or `type`, I grokked the concept. Thanks, I'm gonna try it soon


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
iilyak commented on pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#issuecomment-683781092


   > @iilyak I changed my mind about moduletags, you were right, even if I'd prefer a moduletag in every test module.
   
   Sorry I was not clear. I am not against using moduletags. I was against changing existent ones. Your previous version of the PR had changes like `@moduletag :all_docs` -> `@moduletag :something_else`. I am totally fine with the following:
   
   ```
   @moduletag :all_docs
   @moduletag kind: :single_node
   ```
   
   After re-reading my following passage I do realize how confusing it sounds. Sorry about that.
   > Also I think we shouldn't change @moduletag but use @tag instead for tagging by setup. 
   
   The `moduletag` is better since it allows the developer to keep less things in mind when adding new tests. Again I am very sorry for having you changing it back and force.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] dottorblaster edited a comment on pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
dottorblaster edited a comment on pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#issuecomment-683468123


   @iilyak I changed my mind about moduletags, you were right, even if I'd prefer a moduletag in every test module. Going forth, I tagged every test with its corresponding setup, based on my understanding of the test suite and of CouchDB. Could you review this?
   
   Once this is ready I think we can start experimenting with splitting tests execution inside Makefile, that can be done with (e.g.) `mix test --trace --only kind:single_node $(EXUNIT_OPTS)`


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
iilyak commented on pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#issuecomment-682490951


   The purpose of tagging into groups proposed in the issue https://github.com/apache/couchdb/issues/1885 is to be able to split single `mix test` run here https://github.com/apache/couchdb/blob/master/Makefile#L230:L233 into multiple. Something like:
   ```
   mix test --trace --include single_node:true $(EXUNIT_OPTS)'
   mix test --trace --include degraded_cluster:true $(EXUNIT_OPTS)'
   mix test --trace --include cluster:true $(EXUNIT_OPTS)'
   mix test --trace --include performance:true $(EXUNIT_OPTS)'
   mix test --trace --include property_based:true $(EXUNIT_OPTS)'
   ```
   
   Notice that the tests are grouped by setup type they require. This is in order to put the fast tests before the slow ones. This would shorten development cycle. Currently the tests which require 
   single_node setup might be scheduled in the end of a test run. Which means that even though they are fastest to run the developer would have to wait while most of the test suite would complete (in Jenkins).
   
   Also I think we shouldn't change `@moduletag` but use `@tag` instead for tagging by setup. I didn't check but there might be a way to use tags like so `@tag setup: :single_node`:
   ```
   mix test --trace --include setup:single_node $(EXUNIT_OPTS)'
   mix test --trace --include setup:degraded_cluster $(EXUNIT_OPTS)'
   mix test --trace --include setup:cluster $(EXUNIT_OPTS)'
   mix test --trace --include setup:performance $(EXUNIT_OPTS)'
   mix test --trace --include setup:property_based $(EXUNIT_OPTS)'
   ``` 
    


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
iilyak commented on pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#issuecomment-682491740


   Or maybe instead of `@tag setup: :single_node` we should use `@tag kind: :single_node`. Naming is hard...


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] dottorblaster merged pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
dottorblaster merged pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] dottorblaster commented on pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
dottorblaster commented on pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#issuecomment-682480210


   @iilyak can I have a feedback on this? The next step would be tagging single test cases with the tags you listed into the issue, right?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on a change in pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#discussion_r480324825



##########
File path: test/elixir/test/partition_all_docs_test.exs
##########
@@ -6,6 +6,9 @@ defmodule PartitionAllDocsTest do
   Test Partition functionality for for all_docs
   """
 
+  @moduletag :partition
+  @moduletag kind: :cluster

Review comment:
       never mind `cluster` is fine.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] dottorblaster commented on pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
dottorblaster commented on pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#issuecomment-683437841


   As once great dev once said, "maybe we should hire a poet to name things" - cit. @riquito
   
   Be it `setup`, `kind` or `type`, I grokked the concept. Thanks, I'm gonna try it soon


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on a change in pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#discussion_r480323507



##########
File path: test/elixir/test/partition_crud_test.exs
##########
@@ -1,6 +1,9 @@
 defmodule PartitionCrudTest do
   use CouchTestCase
 
+  @moduletag :partition
+  @moduletag kind: :cluster

Review comment:
       This should be :single_node. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on a change in pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#discussion_r480323373



##########
File path: test/elixir/test/partition_all_docs_test.exs
##########
@@ -6,6 +6,9 @@ defmodule PartitionAllDocsTest do
   Test Partition functionality for for all_docs
   """
 
+  @moduletag :partition
+  @moduletag kind: :cluster

Review comment:
       This should be `:single_node`. Partition feature is not about partitioned cluster it is about [forming logical database  partitions by using a partition key](https://docs.couchdb.org/en/master/partitioned-dbs/index.html).




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on a change in pull request #3110: Tag elixir test modules in meaningful groups

Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #3110:
URL: https://github.com/apache/couchdb/pull/3110#discussion_r480323950



##########
File path: test/elixir/test/partition_design_docs_test.exs
##########
@@ -5,6 +5,9 @@ defmodule PartitionDesignDocsTest do
   Test Partition functionality for partition design docs
   """
 
+  @moduletag :partition
+  @moduletag kind: :cluster

Review comment:
       This should be :single_node. 

##########
File path: test/elixir/test/partition_mango_test.exs
##########
@@ -5,6 +5,10 @@ defmodule PartitionMangoTest do
   @moduledoc """
   Test Partition functionality for mango
   """
+
+  @moduletag :partition
+  @moduletag kind: :cluster

Review comment:
       This should be :single_node. 

##########
File path: test/elixir/test/partition_size_limit_test.exs
##########
@@ -5,6 +5,9 @@ defmodule PartitionSizeLimitTest do
   Test Partition size limit functionality
   """
 
+  @moduletag :partition
+  @moduletag kind: :cluster

Review comment:
       This should be :single_node. 

##########
File path: test/elixir/test/partition_size_test.exs
##########
@@ -4,6 +4,9 @@ defmodule PartitionSizeTest do
   @moduledoc """
   Test Partition size functionality
   """
+  
+  @moduletag :partition
+  @moduletag kind: :cluster

Review comment:
       This should be :single_node. 

##########
File path: test/elixir/test/partition_view_test.exs
##########
@@ -5,6 +5,9 @@ defmodule ViewPartitionTest do
   @moduledoc """
   Test Partition functionality for views
   """
+  
+  @moduletag :partition
+  @moduletag kind: :cluster

Review comment:
       This should be :single_node. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org