Re: [PR] Fix mango tests using custom db name [couchdb]
pgj commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1864135984 ## Makefile: ## @@ -339,7 +339,7 @@ mango-test: devclean all --admin=adm:pass \ --no-eval "\ COUCH_USER=adm COUCH_PASS=pass \ -src/mango/.venv/bin/nose2 -s src/mango/test -c src/mango/unittest.cfg $(MANGO_TEST_OPTS)" +src/mango/.venv/bin/nose2 -F -s src/mango/test -c src/mango/unittest.cfg $(MANGO_TEST_OPTS)" Review Comment: > Comment out some test cases and you'll get different test results. That is what one would not want to do. Commenting test cases in and out is what makes the feedback loop slower. > We can remove the `-F` if it is not needed: https://github.com/apache/couchdb/pull/5348/files?w=1 I have not had the time to try this new approach, maybe it works. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
jiahuili430 commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1864090081 ## Makefile: ## @@ -339,7 +339,7 @@ mango-test: devclean all --admin=adm:pass \ --no-eval "\ COUCH_USER=adm COUCH_PASS=pass \ -src/mango/.venv/bin/nose2 -s src/mango/test -c src/mango/unittest.cfg $(MANGO_TEST_OPTS)" +src/mango/.venv/bin/nose2 -F -s src/mango/test -c src/mango/unittest.cfg $(MANGO_TEST_OPTS)" Review Comment: > Same error message, but bailed on different test cases. The error message is as follows: ```bash Traceback (most recent call last): File "/Users/jiahuili/src/a/src/mango/test/16-index-selectors-test.py", line 214, in test_uses_partial_index_for_query_selector docs = self.db.find(selector, use_index="Selected", fields=["_id", "location"]) File "/Users/jiahuili/src/a/src/mango/test/mango.py", line 317, in find r.raise_for_status() ~~^^ File "/Users/jiahuili/src/a/src/mango/.venv/lib/python3.13/site-packages/requests/models.py", line 960, in raise_for_status raise HTTPError(http_error_msg, response=self) requests.exceptions.HTTPError: 500 Server Error: Internal Server Error for url: http://127.0.0.1:15984/mango_test_480b738790f748d9b5f3c5c17202d501/_find ``` Sometimes it fails on `test_uses_partial_index_for_query_selector` and sometimes it fails on `test_uses_partial_index_with_different_selector`. Comment out some test cases and you'll get different test results. We can remove the `-F` if it is not needed: https://github.com/apache/couchdb/pull/5348/files?w=1 -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
jiahuili430 commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1864090081 ## Makefile: ## @@ -339,7 +339,7 @@ mango-test: devclean all --admin=adm:pass \ --no-eval "\ COUCH_USER=adm COUCH_PASS=pass \ -src/mango/.venv/bin/nose2 -s src/mango/test -c src/mango/unittest.cfg $(MANGO_TEST_OPTS)" +src/mango/.venv/bin/nose2 -F -s src/mango/test -c src/mango/unittest.cfg $(MANGO_TEST_OPTS)" Review Comment: > Same error message, but bailed on different test cases. The error message is as follows: ```bash Traceback (most recent call last): File "/Users/jiahuili/src/a/src/mango/test/16-index-selectors-test.py", line 214, in test_uses_partial_index_for_query_selector docs = self.db.find(selector, use_index="Selected", fields=["_id", "location"]) File "/Users/jiahuili/src/a/src/mango/test/mango.py", line 317, in find r.raise_for_status() ~~^^ File "/Users/jiahuili/src/a/src/mango/.venv/lib/python3.13/site-packages/requests/models.py", line 960, in raise_for_status raise HTTPError(http_error_msg, response=self) requests.exceptions.HTTPError: 500 Server Error: Internal Server Error for url: http://127.0.0.1:15984/mango_test_480b738790f748d9b5f3c5c17202d501/_find ``` Sometimes it fails on `test_uses_partial_index_for_query_selector` and sometimes it fails on `test_uses_partial_index_with_different_selector`. Comment out some test cases and you'll get different test results. We can remove the `-F` if it is not needed: https://github.com/apache/couchdb/pull/5348/files -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
pgj commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1863840544 ## Makefile: ## @@ -339,7 +339,7 @@ mango-test: devclean all --admin=adm:pass \ --no-eval "\ COUCH_USER=adm COUCH_PASS=pass \ -src/mango/.venv/bin/nose2 -s src/mango/test -c src/mango/unittest.cfg $(MANGO_TEST_OPTS)" +src/mango/.venv/bin/nose2 -F -s src/mango/test -c src/mango/unittest.cfg $(MANGO_TEST_OPTS)" Review Comment: > 0.85 sec vs 50 sec to run the entire test suite. Sure, I know that running the Mango test suite in the past required around a minute to run. In my opinion, that is acceptable, since it is still does not take tens of minutes or many hours. > Different combinations of test cases will result in the same failure message but fail on different test cases. I am not sure if I got you on this. If the "fail fast" mode stops at the first failing test, how can it result in the failure message if there are multiple failures? > `-F, --failfast` allows us to identify the issue quickly, speed up the feedback loop, and make debugging easier. Note that you could tell the names of the modules or specific test cases on the command line when invoking `nose2`. This is also exposed in the general `Makefile` of the repository, for the `mango-test` target as the `MANGO_TEST_OPTS` variable. With the help of this, one could easily limit the current run to the tests that one has to run for keeping the feedback loop short. With the regards to finding the tests to run, it is okay to run the whole suite once and take the cost of the full run. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
jiahuili430 commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1860974792 ## Makefile: ## @@ -339,7 +339,7 @@ mango-test: devclean all --admin=adm:pass \ --no-eval "\ COUCH_USER=adm COUCH_PASS=pass \ -src/mango/.venv/bin/nose2 -s src/mango/test -c src/mango/unittest.cfg $(MANGO_TEST_OPTS)" +src/mango/.venv/bin/nose2 -F -s src/mango/test -c src/mango/unittest.cfg $(MANGO_TEST_OPTS)" Review Comment: While working on this, I even changed `unittest.cfg` to run `16-index-selectors-test.py` only. Because I keep getting 500 errors due to this test file. Different combinations of test cases will result in the same failure message but fail on different test cases. 0.85 sec vs 50 sec to run the entire test suite. `-F, --failfast` allows us to identify the issue quickly, speed up the feedback loop, and make debugging easier. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
pgj commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1860818125 ## Makefile: ## @@ -339,7 +339,7 @@ mango-test: devclean all --admin=adm:pass \ --no-eval "\ COUCH_USER=adm COUCH_PASS=pass \ -src/mango/.venv/bin/nose2 -s src/mango/test -c src/mango/unittest.cfg $(MANGO_TEST_OPTS)" +src/mango/.venv/bin/nose2 -F -s src/mango/test -c src/mango/unittest.cfg $(MANGO_TEST_OPTS)" Review Comment: What does `-F` buy for us? The Mango tests are quick to run, and sometimes perhaps it is good to see if there are other errors besides the one that failed the whole run. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
jiahuili430 merged PR #5341: URL: https://github.com/apache/couchdb/pull/5341 -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
jiahuili430 commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1858801200 ## src/mango/test/16-index-selectors-test.py: ## @@ -189,6 +189,7 @@ def test_uses_partial_index_with_non_indexable_selector(self): @unittest.skipUnless(mango.has_text_service(), "requires text service") class IndexSelectorText(mango.DbPerClass): def setUp(self): +self.db = mango.Database(f"{mango.random_db_name()}_{str(self).split()[0]}") Review Comment: Thank you all for the suggestions and comments. - `recreate()` is used in `classmethod` `setUpClass()`, such as `UserDocsTests`, `NumStringTests`, etc., which is difficult to modify. - `recreat_random()`, simple solution, but deleting the old database becomes a problem. - So finally decided to use `setUp()` and `tearDown()`. Use a boolean variable `db_per_test` to control whether to use class db or create/delete a custom db for each test. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
jiahuili430 commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1858762292 ## src/mango/test/mango.py: ## @@ -107,24 +104,19 @@ def create(self, q=1, n=1, partitioned=False): def delete(self): r = self.sess.delete(self.url) +r.raise_for_status() def recreate(self): -NUM_TRIES = 10 - -for k in range(NUM_TRIES): Review Comment: Tried many times, `k` is only equal to 0 and 1, so I guess maybe we can revert them back. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
jiahuili430 commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1858762292 ## src/mango/test/mango.py: ## @@ -107,24 +104,19 @@ def create(self, q=1, n=1, partitioned=False): def delete(self): r = self.sess.delete(self.url) +r.raise_for_status() def recreate(self): -NUM_TRIES = 10 - -for k in range(NUM_TRIES): Review Comment: Tried many times, `k` is only equal to 0 and 1, so I decided to revert back. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
pgj commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1857313105 ## src/mango/test/16-index-selectors-test.py: ## @@ -189,6 +189,7 @@ def test_uses_partial_index_with_non_indexable_selector(self): @unittest.skipUnless(mango.has_text_service(), "requires text service") class IndexSelectorText(mango.DbPerClass): def setUp(self): +self.db = mango.Database(f"{mango.random_db_name()}_{str(self).split()[0]}") Review Comment: Virtually, that is still about recreating the database, in my opinion. But I agree that `create_random` would capture better the adjusted semantics for option 4. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
jaydoane commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1857027280 ## src/mango/test/16-index-selectors-test.py: ## @@ -189,6 +189,7 @@ def test_uses_partial_index_with_non_indexable_selector(self): @unittest.skipUnless(mango.has_text_service(), "requires text service") class IndexSelectorText(mango.DbPerClass): def setUp(self): +self.db = mango.Database(f"{mango.random_db_name()}_{str(self).split()[0]}") Review Comment: Option 4 seems like the simplest approach, although maybe consider a name change since it wouldn't be "recreating" the db if it has a different name. Maybe just `create_random` or something like that? -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
pgj commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1856970437 ## src/mango/test/16-index-selectors-test.py: ## @@ -189,6 +189,7 @@ def test_uses_partial_index_with_non_indexable_selector(self): @unittest.skipUnless(mango.has_text_service(), "requires text service") class IndexSelectorText(mango.DbPerClass): def setUp(self): +self.db = mango.Database(f"{mango.random_db_name()}_{str(self).split()[0]}") self.db.recreate() Review Comment: Yes, the scope of that commit was only limited to solving the infinite loop of retries with recursion. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
pgj commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1856969017 ## src/mango/test/16-index-selectors-test.py: ## @@ -189,6 +189,7 @@ def test_uses_partial_index_with_non_indexable_selector(self): @unittest.skipUnless(mango.has_text_service(), "requires text service") class IndexSelectorText(mango.DbPerClass): def setUp(self): +self.db = mango.Database(f"{mango.random_db_name()}_{str(self).split()[0]}") Review Comment: I would vote for option 4. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
pgj commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1856970437 ## src/mango/test/16-index-selectors-test.py: ## @@ -189,6 +189,7 @@ def test_uses_partial_index_with_non_indexable_selector(self): @unittest.skipUnless(mango.has_text_service(), "requires text service") class IndexSelectorText(mango.DbPerClass): def setUp(self): +self.db = mango.Database(f"{mango.random_db_name()}_{str(self).split()[0]}") self.db.recreate() Review Comment: Yes, that scope of that commit was only limited to solving the infinite loop of retries with recursion. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
iilyak commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1856700329 ## src/mango/test/16-index-selectors-test.py: ## @@ -189,6 +189,7 @@ def test_uses_partial_index_with_non_indexable_selector(self): @unittest.skipUnless(mango.has_text_service(), "requires text service") class IndexSelectorText(mango.DbPerClass): def setUp(self): +self.db = mango.Database(f"{mango.random_db_name()}_{str(self).split()[0]}") Review Comment: All time based approaches are flowed. They don't solve the problem. They introduce flakiness. The options I see: 1. restructure all tests so they are not using recreate. They would always start from scratch and create fresh databaase with uniq name. 2. update dreyfus_index_manager.erl to handle `cleanup` differently 3. modify clouseau so it handle rapid re-creation better 4. change recreate function so it uses new name on each retry -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
jiahuili430 commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1854797911 ## src/mango/test/16-index-selectors-test.py: ## @@ -189,6 +189,7 @@ def test_uses_partial_index_with_non_indexable_selector(self): @unittest.skipUnless(mango.has_text_service(), "requires text service") class IndexSelectorText(mango.DbPerClass): def setUp(self): +self.db = mango.Database(f"{mango.random_db_name()}_{str(self).split()[0]}") Review Comment: If using the same db name, `0.13` is the shortest time to avoid `500` locally. This approach also increased the overall Mango test time to `60.730s`, compared to `49.936s` using custom database name. ```diff def recreate(self): -NUM_TRIES = 10 +NUM_TRIES = 3 -for k in range(NUM_TRIES): +for k in range(1, NUM_TRIES): r = self.sess.get(self.url) if r.status_code == 200: db_info = r.json() docs = db_info["doc_count"] + db_info["doc_del_count"] if docs == 0: # db exists and it is empty -- exit condition is met return self.delete() +time.sleep(k * 0.13) self.create() -time.sleep(k * 0.1) raise Exception( "Failed to recreate the database after {} tries".format(NUM_TRIES) ) ``` -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
jiahuili430 commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1854797911 ## src/mango/test/16-index-selectors-test.py: ## @@ -189,6 +189,7 @@ def test_uses_partial_index_with_non_indexable_selector(self): @unittest.skipUnless(mango.has_text_service(), "requires text service") class IndexSelectorText(mango.DbPerClass): def setUp(self): +self.db = mango.Database(f"{mango.random_db_name()}_{str(self).split()[0]}") Review Comment: If using the same db name, `0.13` is the shortest time I need to avoid `500` locally. This approach also increased the overall Mango test time to `60.730s`, compared to `49.936s` using custom database name. ```diff def recreate(self): -NUM_TRIES = 10 +NUM_TRIES = 3 -for k in range(NUM_TRIES): +for k in range(1, NUM_TRIES): r = self.sess.get(self.url) if r.status_code == 200: db_info = r.json() docs = db_info["doc_count"] + db_info["doc_del_count"] if docs == 0: # db exists and it is empty -- exit condition is met return self.delete() +time.sleep(k * 0.13) self.create() -time.sleep(k * 0.1) raise Exception( "Failed to recreate the database after {} tries".format(NUM_TRIES) ) ``` -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
jiahuili430 commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1854760661 ## src/mango/test/16-index-selectors-test.py: ## @@ -189,6 +189,7 @@ def test_uses_partial_index_with_non_indexable_selector(self): @unittest.skipUnless(mango.has_text_service(), "requires text service") class IndexSelectorText(mango.DbPerClass): def setUp(self): +self.db = mango.Database(f"{mango.random_db_name()}_{str(self).split()[0]}") self.db.recreate() Review Comment: Thank you for the link! -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
jaydoane commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1854742220 ## src/mango/test/16-index-selectors-test.py: ## @@ -189,6 +189,7 @@ def test_uses_partial_index_with_non_indexable_selector(self): @unittest.skipUnless(mango.has_text_service(), "requires text service") class IndexSelectorText(mango.DbPerClass): def setUp(self): +self.db = mango.Database(f"{mango.random_db_name()}_{str(self).split()[0]}") self.db.recreate() Review Comment: There was [recent work](https://github.com/apache/couchdb/commit/eb6a74d5603d4bac1d77fc895946ffbdd99f4b97) done on the `recreate` function, but it didn't seem to address the raciness of reusing db names. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix mango tests using custom db name [couchdb]
iilyak commented on code in PR #5341: URL: https://github.com/apache/couchdb/pull/5341#discussion_r1854706374 ## src/mango/test/16-index-selectors-test.py: ## @@ -189,6 +189,7 @@ def test_uses_partial_index_with_non_indexable_selector(self): @unittest.skipUnless(mango.has_text_service(), "requires text service") class IndexSelectorText(mango.DbPerClass): def setUp(self): +self.db = mango.Database(f"{mango.random_db_name()}_{str(self).split()[0]}") Review Comment: There are many tests files which use `self.db.recreate()` did you consider modifying that function? -- 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. To unsubscribe, e-mail: notifications-unsubscr...@couchdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org