Skip to content
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.

Sign up for GitHub

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

Skip failing custom dc tests. #4820

Merged
merged 1 commit into from
Dec 27, 2024
Merged

Skip failing custom dc tests. #4820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions server/webdriver/cdc_tests/autopush/cdc_base_webdriver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
from server.webdriver.base import WebdriverBaseTest
from server.webdriver.cdc_tests.base import CDC_AUTOPUSH_URL

# Shared tests that don't work in custom DC are skipped with this reason.
# Any skipped test should be noted in b/386405593, fixed and unskipped subsequently.
SKIPPED_TEST_REASON = "Not working in custom DC. See b/386405593"


class CdcAutopushTestBase(WebdriverBaseTest):
"""Base class for CDC Autopush webdriver tests."""
Expand Down
8 changes: 8 additions & 0 deletions server/webdriver/cdc_tests/autopush/place_explorer_i18n_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,19 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import unittest

from server.webdriver.cdc_tests.autopush.cdc_base_webdriver import \
CdcAutopushTestBase
from server.webdriver.cdc_tests.autopush.cdc_base_webdriver import \
SKIPPED_TEST_REASON
from server.webdriver.shared_tests.place_explorer_i18n_test import \
PlaceI18nExplorerTestMixin


class TestPlaceExplorerI18n(PlaceI18nExplorerTestMixin, CdcAutopushTestBase):
"""Class to test the i18n place explorer page for Custom DC. Tests come from PlaceI18nExplorerTestMixin."""

@unittest.skip(reason=SKIPPED_TEST_REASON)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a quick note: we already have an approach for skipping unit tests in custom DC. You can remove the test from the shared_tests mixin and add it directly to the main DC /tests/ package instead. Here's an example example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the note. In these 2 cases I don't know if we actually want to remove them or fix and keep them in custom DC. I'll work with Gabe in the new year to figure it out.

Skipping them explicitly for now to stop the alerts until then.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think what is done here is better because if this test passes on base dc we should keep it! We definitely should fix this test for custom dc. I like the way keyur did it here!
Also I think we ve had enough instances of breaking auto push cdc with these webdriver tests. We should really try to have these run on a local cdc as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good—skipping for now makes sense

Copy link
Contributor

Choose a reason for hiding this comment

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

@gmechali agreed. is testing against autopush helpful, or should these tests only run against a local cdc?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd argue it's still useful since it allows us to test what's deployed in autopush and catch bugs pre releases
But yeah local would probably be a bit more valuable since we d catch it even earlier

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks both for chiming in!

def test_explorer_redirect_place_explorer_keeps_locale(self):
super().test_explorer_redirect_place_explorer_keeps_locale()
8 changes: 8 additions & 0 deletions server/webdriver/cdc_tests/autopush/place_explorer_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,19 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import unittest

from server.webdriver.cdc_tests.autopush.cdc_base_webdriver import \
CdcAutopushTestBase
from server.webdriver.cdc_tests.autopush.cdc_base_webdriver import \
SKIPPED_TEST_REASON
from server.webdriver.shared_tests.place_explorer_test import \
PlaceExplorerTestMixin


class TestPlaceExplorer(PlaceExplorerTestMixin, CdcAutopushTestBase):
"""Class to test the place explorer page for Custom DC. Tests come from PlaceExplorerTestMixin."""

@unittest.skip(reason=SKIPPED_TEST_REASON)
def test_explorer_redirect_place_explorer(self):
super().test_explorer_redirect_place_explorer()
Loading