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.

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

fix: remove feature flags #301

Merged
merged 2 commits into from
Mar 4, 2024
Merged

Conversation

daniel-codecov
Copy link
Contributor

@daniel-codecov daniel-codecov commented Mar 4, 2024

Temporarily remove feature flags since it is querying django models which will be blocking the event loop in asyncio.

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@codecov-staging
Copy link

Test Failures Detected: Due to failing tests, we cannot provide coverage reports at this time.

❌ Failed Test Results:

Completed 1392 tests with 4 failed, 1388 passed and 0 skipped.

View the full list of failed tests
Test Description Failure message
Testsuite: pytest
Test name: services.report.tests.unit.test_sessio
ns_encoded_labels.TestAdjustSession::test_adjust_sessions_partial_cf_o
nly_no_changes[1]
Envs:
-
self = <test_sessions_encoded_labels.TestAdjustSession object at 0x7fbb3126ae90>
sample_first_report = <EditableReport files=1>
sample_first_report_no_encoded_labels = <EditableReport files=1>
mocker = <pytest_mock.plugin.MockFixture object at 0x7fbb2a008e50>
report_idx = 1

@pytest.mark.parametrize("report_idx", [0, 1])
def test_adjust_sessions_partial_cf_only_no_changes(
self,
sample_first_report,
sample_first_report_no_encoded_labels,
mocker,
report_idx,
):
report_under_test = [
sample_first_report,
sample_first_report_no_encoded_labels,
][report_idx]

mocker.patch.object(
USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID,
"check_value",
return_value=True,
)
first_to_merge_session = Session(flags=["enterprise"], id=3)
second_report = Report(
sessions={first_to_merge_session.id: first_to_merge_session}
)
upload = MagicMock(
name="fake_upload",
**{
"report": MagicMock(
name="fake_commit_report",
**{
"code": None,
"commit": MagicMock(
name="fake_commit",
**{"repository": MagicMock(name="fake_repo")}
),
}
)
}
)
current_yaml = UserYaml(
{
"flag_management": {
"individual_flags": [
{
"name": "enterprise",
"carryforward_mode": "labels",
"carryforward": True,
}
]
}
}
)

first_value = self.convert_report_to_better_readable(sample_first_report)
# This makes changes to the not-label-encoded original report, encoding them
assert _adjust_sessions(
report_under_test,
second_report,
first_to_merge_session,
current_yaml,
upload=upload,
) == SessionAdjustmentResult([], [0])
# The after result should always be the encoded labels one
after_result = self.convert_report_to_better_readable(report_under_test)
> assert after_result == first_value
E AssertionError: assert {'archive': {... 'b': 0, ...}} == {'archive': {... 'b': 0, ...}}
E Omitting 2 identical items, use -vv to show
E Differing items:
E {'archive': {'first_file.py': [(1, 14, None, [[0, 0, None, None, None], [3, 7, None, None, None], [2, 14, None, None, ...6, 19, None, [[1, 5, None, None, None], [0, 12, None, None, None], [3, 19, None, None, None]], None, None, ...), ...]}} != {'archive': {'first_file.py': [(1, 14, None, [[0, 0, None, None, None], [3, 7, None, None, None], [2, 14, None, None, ...6, 19, None, [[1, 5, None, None, None], [0, 12, None, None, None], [3, 19, None, None, None]], None, None, ...), ...]}}
E Use -v to get more diff

services/report/tests/unit/test_sessions_encoded_labels.py:665: AssertionError
Testsuite: pytest
Test name: tasks.tests.unit.test_sync_repos_task.
TestSyncReposTaskUnit::test_sync_repos_timeout[True]
Envs:
-
self = <worker.tasks.tests.unit.test_sync_repos_task.TestSyncReposTaskUnit object at 0x7fbb301dd9c0>
mocker = <pytest_mock.plugin.MockFixture object at 0x7fbb2a4f8970>
mock_configuration = <shared.config.ConfigHelper object at 0x7fbb2a527400>
dbsession = <sqlalchemy.orm.session.Session object at 0x7fbb2a5265f0>
mock_owner_provider = <MagicMock name='_get_owner_provider_service_instance()' spec='Github' id='140441845406768'>
mock_redis = <MagicMock name='_get_redis_instance_from_url()' id='140441884174240'>
use_generator = True

@pytest.mark.asyncio
@pytest.mark.parametrize("use_generator", [False, True])
async def test_sync_repos_timeout(
self,
mocker,
mock_configuration,
dbsession,
mock_owner_provider,
mock_redis,
use_generator,
):
mocker.patch.object(
LIST_REPOS_GENERATOR_BY_OWNER_ID, "check_value", return_value=use_generator
)

repos = [RepositoryFactory.create(private=True) for _ in range(10)]
dbsession.add_all(repos)
dbsession.flush()
user = OwnerFactory.create(
organizations=[], permission=sorted([r.repoid for r in repos])
)
assert len(user.permission) > 0
dbsession.add(user)
dbsession.flush()

if use_generator:
mock_owner_provider.list_repos_generator.side_effect = (
SoftTimeLimitExceeded()
)
else:
mock_owner_provider.list_repos.side_effect = SoftTimeLimitExceeded()

> with pytest.raises(SoftTimeLimitExceeded):
E Failed: DID NOT RAISE <class 'billiard.exceptions.SoftTimeLimitExceeded'>

tasks/tests/unit/test_sync_repos_task.py:820: Failed
Testsuite: pytest
Test name: tasks.tests.unit.test_sync_repos_task.
TestSyncReposTaskUnit::test_sync_repos_using_integration[True]
Envs
:
-
self = <worker.tasks.tests.unit.test_sync_repos_task.TestSyncReposTaskUnit object at 0x7fbb301de950>
mocker = <pytest_mock.plugin.MockFixture object at 0x7fbb2b9d2590>
dbsession = <sqlalchemy.orm.session.Session object at 0x7fbb2b9d1d50>
mock_owner_provider = <MagicMock name='_get_owner_provider_service_instance()' spec='Github' id='140441871036288'>
mock_redis = <MagicMock name='_get_redis_instance_from_url()' id='140441870679584'>
use_generator = True

@pytest.mark.asyncio
@pytest.mark.parametrize("use_generator", [False, True])
@respx.mock
@reuse_cassette(
"tasks/tests/unit/cassetes/test_sync_repos_task/TestSyncReposTaskUnit/test_sync_repos_using_integration.yaml"
)
@pytest.mark.django_db(databases={"default"})
async def test_sync_repos_using_integration(
self,
mocker,
dbsession,
mock_owner_provider,
mock_redis,
use_generator,
):
mocker.patch.object(
LIST_REPOS_GENERATOR_BY_OWNER_ID, "check_value", return_value=use_generator
)

if use_generator:
respx.post("https://api\.github\.com/graphql\"\)\.mock\(
httpx.Response(
status_code=200,
content='{"data":{"viewer":{"repositories":{"totalCount": 4}}}}',
headers={"Content-Type": "application/json"},
)
)

token = "ecd73a086eadc85db68747a66bdbd662a785a072"
user = OwnerFactory.create(
organizations=[],
service="github",
username="1nf1n1t3l00p",
unencrypted_oauth_token=token,
permission=[],
service_id="45343385",
)
dbsession.add(user)

def repo_obj(service_id, name, language, private, branch, using_integration):
return {
"owner": {
"service_id": "test-owner-service-id",
"username": "test-owner-username",
},
"repo": {
"service_id": service_id,
"name": name,
"language": language,
"private": private,
"branch": branch,
},
"_using_integration": using_integration,
}

mock_repos = [
repo_obj("159089634", "pytest", "python", False, "main", True),
repo_obj("164948070", "spack", "python", False, "develop", False),
repo_obj("213786132", "pub", "dart", False, "master", None),
repo_obj("555555555", "soda", "python", False, "main", None),
]

# Mock GitHub response for repos that are visible to our app
if use_generator:
mock_owner_provider.list_repos_using_installation_generator.return_value = (
AsyncIterator([mock_repos])
)
else:
mock_owner_provider.list_repos_using_installation.return_value = mock_repos

# Three of the four repositories we can see are already in the database.
# Will we update `using_integration` correctly?
preseeded_repos = []
for repo in mock_repos[:-1]:
preseeded_repos.append(
RepositoryFactory.create(
private=repo["repo"]["private"],
name=repo["repo"]["name"],
using_integration=repo["_using_integration"],
service_id=repo["repo"]["service_id"],
owner=user,
)
)

for repo in preseeded_repos:
dbsession.add(repo)
dbsession.flush()

await SyncReposTask().run_async(
dbsession, ownerid=user.ownerid, using_integration=True
)
dbsession.commit()

repos = (
dbsession.query(Repository)
.filter(
Repository.service_id.in_(
(repo["repo"]["service_id"] for repo in mock_repos)
)
)
.all()
)

# We pre-seeded 3 repos in the database, but we should have added the
# 4th based on our GitHub response
> assert len(repos) == 4
E assert 3 == 4
E + where 3 = len([Repo<817>, Repo<818>, Repo<819>])

tasks/tests/unit/test_sync_repos_task.py:673: AssertionError
Testsuite: pytest
Test name: tasks.tests.unit.test_sync_repos_task.
TestSyncReposTaskUnit::test_insert_repo_and_call_repo_sync_languages_u
sing_integration[True]
Envs:
-
self = <worker.tasks.tests.unit.test_sync_repos_task.TestSyncReposTaskUnit object at 0x7fbb30436a70>
mocker = <pytest_mock.plugin.MockFixture object at 0x7fbb2a8158d0>
dbsession = <sqlalchemy.orm.session.Session object at 0x7fbb2a814ee0>
mock_owner_provider = <MagicMock name='_get_owner_provider_service_instance()' spec='Github' id='140441844355840'>
mock_redis = <MagicMock name='_get_redis_instance_from_url()' id='140441843966608'>
use_generator = True

@pytest.mark.asyncio
@pytest.mark.parametrize("use_generator", [False, True])
@respx.mock
@reuse_cassette(
"tasks/tests/unit/cassetes/test_sync_repos_task/TestSyncReposTaskUnit/test_sync_repos_using_integration.yaml"
)
async def test_insert_repo_and_call_repo_sync_languages_using_integration(
self,
mocker,
dbsession,
mock_owner_provider,
mock_redis,
use_generator,
):
mocked_app = mocker.patch.object(
SyncReposTask,
"app",
tasks={
sync_repo_languages_task_name: mocker.MagicMock(),
},
)

mocker.patch.object(
LIST_REPOS_GENERATOR_BY_OWNER_ID, "check_value", return_value=use_generator
)

if use_generator:
respx.post("https://api\.github\.com/graphql\"\)\.mock\(
httpx.Response(
status_code=200,
content='{"data":{"viewer":{"repositories":{"totalCount": 4}}}}',
headers={"Content-Type": "application/json"},
)
)

token = "ecd73a086eadc85db68747a66bdbd662a785a072"
user = OwnerFactory.create(
organizations=[],
service="github",
username="1nf1n1t3l00p",
unencrypted_oauth_token=token,
permission=[],
service_id="45343385",
)
dbsession.add(user)

def repo_obj(service_id, name, language, private, branch, using_integration):
return {
"owner": {
"service_id": "test-owner-service-id",
"username": "test-owner-username",
},
"repo": {
"service_id": service_id,
"name": name,
"language": language,
"private": private,
"branch": branch,
},
"_using_integration": using_integration,
}

mock_repos = [
repo_obj("159089634", "pytest", "python", False, "main", True),
repo_obj("164948070", "spack", "python", False, "develop", False),
repo_obj("213786132", "pub", "dart", False, "master", None),
repo_obj("555555555", "soda", "python", False, "main", None),
]

# Mock GitHub response for repos that are visible to our app
if use_generator:
mock_owner_provider.list_repos_using_installation_generator.return_value = (
AsyncIterator([mock_repos])
)
else:
mock_owner_provider.list_repos_using_installation.return_value = mock_repos

# Three of the four repositories we can see are already in the database.
# Will we update `using_integration` correctly?
preseeded_repos = []
for repo in mock_repos[:-1]:
preseeded_repos.append(
RepositoryFactory.create(
private=repo["repo"]["private"],
name=repo["repo"]["name"],
using_integration=repo["_using_integration"],
service_id=repo["repo"]["service_id"],
owner=user,
)
)

for repo in preseeded_repos:
dbsession.add(repo)
dbsession.flush()

await SyncReposTask().run_async(
dbsession, ownerid=user.ownerid, using_integration=True
)
dbsession.commit()

repos = (
dbsession.query(Repository)
.filter(
Repository.service_id.in_(
(repo["repo"]["service_id"] for repo in mock_repos)
)
)
.all()
)

# We pre-seeded 3 repos in the database, but we should have added the
# 4th based on our GitHub response
> assert len(repos) == 4
E assert 3 == 4
E + where 3 = len([Repo<870>, Repo<871>, Repo<872>])

tasks/tests/unit/test_sync_repos_task.py:1002: AssertionError

Copy link

codecov bot commented Mar 4, 2024

Codecov Report

Attention: Patch coverage is 90.00000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 98.10%. Comparing base (9dcb955) to head (7b9fcf4).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #301      +/-   ##
==========================================
- Coverage   98.11%   98.10%   -0.01%     
==========================================
  Files         414      414              
  Lines       32772    32764       -8     
==========================================
- Hits        32153    32142      -11     
- Misses        619      622       +3     
Flag Coverage Δ
integration 98.07% <90.00%> (-0.06%) ⬇️
latest-uploader-overall 98.07% <90.00%> (-0.06%) ⬇️
unit 98.07% <90.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
NonTestCode 96.09% <72.72%> (-0.03%) ⬇️
OutsideTasks 97.88% <62.50%> (-0.02%) ⬇️
Files Coverage Δ
tasks/sync_repos.py 96.84% <100.00%> (-0.15%) ⬇️
tasks/tests/unit/test_sync_repos_task.py 100.00% <100.00%> (ø)
rollouts/__init__.py 72.72% <62.50%> (-27.28%) ⬇️
Related Entrypoints
run/app.tasks.sync_repos.SyncRepos

@codecov-qa
Copy link

codecov-qa bot commented Mar 4, 2024

Codecov Report

Attention: Patch coverage is 90.00000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 98.07%. Comparing base (9dcb955) to head (7b9fcf4).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #301      +/-   ##
==========================================
- Coverage   98.12%   98.07%   -0.06%     
==========================================
  Files         383      383              
  Lines       32071    32063       -8     
==========================================
- Hits        31471    31445      -26     
- Misses        600      618      +18     
Flag Coverage Δ
integration 98.07% <90.00%> (-0.06%) ⬇️
latest-uploader-overall 98.07% <90.00%> (-0.06%) ⬇️
unit 98.07% <90.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
NonTestCode 96.15% <72.72%> (-0.04%) ⬇️
OutsideTasks 97.88% <62.50%> (-0.02%) ⬇️
Files Coverage Δ
tasks/sync_repos.py 96.29% <100.00%> (-0.68%) ⬇️
tasks/tests/unit/test_sync_repos_task.py 96.43% <100.00%> (-3.57%) ⬇️
rollouts/__init__.py 72.72% <62.50%> (-27.28%) ⬇️

Copy link

codecov-public-qa bot commented Mar 4, 2024

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (9dcb955) 98.12% compared to head (7b9fcf4) 98.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #301      +/-   ##
==========================================
- Coverage   98.12%   98.07%   -0.06%     
==========================================
  Files         383      383              
  Lines       32071    32063       -8     
==========================================
- Hits        31471    31445      -26     
- Misses        600      618      +18     
Flag Coverage Δ
integration 98.07% <90.00%> (-0.06%) ⬇️
latest-uploader-overall 98.07% <90.00%> (-0.06%) ⬇️
unit 98.07% <90.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
NonTestCode 96.15% <72.72%> (-0.04%) ⬇️
OutsideTasks 97.88% <62.50%> (-0.02%) ⬇️
Files Coverage Δ
tasks/sync_repos.py 96.29% <100.00%> (-0.68%) ⬇️
tasks/tests/unit/test_sync_repos_task.py 96.43% <100.00%> (-3.57%) ⬇️
rollouts/__init__.py 72.72% <62.50%> (-27.28%) ⬇️

pass

def check_value(self, id, default=False):
if id == 16621196 or id == 16273544:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

gio's overrides

Copy link
Contributor

Choose a reason for hiding this comment

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

lolol, perhaps would be useful to a comment in the code as to what this is

@daniel-codecov daniel-codecov merged commit a4762c4 into main Mar 4, 2024
14 of 26 checks passed
@daniel-codecov daniel-codecov deleted the daniel/temp-fix-sync-feature-flag branch March 4, 2024 23:18
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants