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

dev: Add Most F Rules to Worker #496

Merged
merged 2 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 6 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,18 @@ lint.install:
echo "Installing..."
pip install -Iv ruff

# The preferred method (for now) w.r.t. fixable rules is to manually update the makefile
# with --fix and re-run 'make lint.' Since ruff is constantly adding rules this is a slight
# amount of "needed" friction imo.
lint.run:
ruff check --select F401 --select I . --fix
ruff check
ruff format

lint.check:
echo "Linting..."
ruff check
echo "Formatting..."
ruff format --check
echo "Sorting..."
ruff check --select F401 --select I .

build.requirements:
# if docker pull succeeds, we have already build this version of
Expand Down
63 changes: 63 additions & 0 deletions ruff.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# Exclude a variety of commonly ignored directories.
exclude = [
".bzr",
".direnv",
".eggs",
".git",
".git-rewrite",
".hg",
".ipynb_checkpoints",
".mypy_cache",
".nox",
".pants.d",
".pyenv",
".pytest_cache",
".pytype",
".ruff_cache",
".svn",
".tox",
".venv",
".vscode",
"__pypackages__",
"_build",
"buck-out",
"build",
"dist",
"node_modules",
"site-packages",
"venv",
]

# Same as Black.
line-length = 88
indent-width = 4

# Assume Python 3.12
target-version = "py312"

[lint]
# Currently only enabled for most F (Pyflakes) and I (isort) rules: https://docs.astral.sh/ruff/rules/
select = ["F", "I"]
ignore = ["F841", "F405", "F403"]

# Allow fix for all enabled rules (when `--fix`) is provided.
# The preferred method (for now) w.r.t. fixable rules is to manually update the makefile
# with --fix and re-run 'make lint'
fixable = ["ALL"]
unfixable = []

# Allow unused variables when underscore-prefixed.
dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$"

[format]
# Like Black, use double quotes for strings.
quote-style = "double"

# Like Black, indent with spaces, rather than tabs.
indent-style = "space"

# Like Black, respect magic trailing commas.
skip-magic-trailing-comma = false

# Like Black, automatically detect the appropriate line ending.
line-ending = "auto"
2 changes: 1 addition & 1 deletion services/notification/notifiers/tests/unit/test_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,7 @@ async def test_notification_exception(
assert result.data_sent is None

@pytest.mark.asyncio
async def test_notification_exception(self, sample_comparison, mocker):
async def test_notification_exception_not_fit(self, sample_comparison, mocker):
notifier = ChecksNotifier(
repository=sample_comparison.head.commit.repository,
title="title",
Expand Down
2 changes: 1 addition & 1 deletion services/notification/notifiers/tests/unit/test_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -4977,7 +4977,7 @@ async def test_build_message_team_plan_customer_all_lines_covered_test_results_e
assert result == expected_result

@pytest.mark.asyncio
async def test_build_message_team_plan_customer_all_lines_covered(
async def test_build_message_team_plan_customer_all_lines_covered_no_third_line(
self,
dbsession,
mock_configuration,
Expand Down
1 change: 0 additions & 1 deletion services/report/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
from shared.utils.sessions import Session, SessionType
from shared.yaml import UserYaml

from database.enums import ReportType
from database.models import Commit, Repository, Upload, UploadError
from database.models.reports import (
AbstractTotals,
Expand Down
26 changes: 0 additions & 26 deletions services/tests/test_flake_detector.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,32 +161,6 @@ def test_flake_consecutive_differing_outcomes_no_main_branch_specified(dbsession
assert flaky_tests[test_id] == {FlakeSymptomType.CONSECUTIVE_DIFF_OUTCOMES}


def test_flake_consecutive_differing_outcomes_no_main_branch_specified(dbsession):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joseph-sentry This test looked to be a copy of the one above it, let me know if we actually wanted to keep

Copy link
Contributor

Choose a reason for hiding this comment

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

if it's an exact copy, it's probably a slip up and can be deleted, but if they differ a bit, then it's probably a name mixup and we should rename and keep

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it was an exact copy :o

repoid = create_repo(
dbsession,
)
commitid = create_commit(dbsession, repoid, "not_main")
reportid = create_report(dbsession, commitid)
reportid2 = create_report(dbsession, commitid)
uploadid = create_upload(dbsession, reportid)
uploadid2 = create_upload(dbsession, reportid2)
test_id = create_test(dbsession, repoid)
ti1 = create_test_instance(
dbsession, test_id, uploadid, str(Outcome.Failure), "failure message"
)
_ = create_test_instance(dbsession, test_id, uploadid2, str(Outcome.Pass), None)

dbfd = DefaultBranchFailureDetector(dbsession, repoid)
umd = UnrelatedMatchesDetector()
dod = DiffOutcomeDetector()

fd = FlakeDetectionEngine(dbsession, repoid, [dbfd, dod, umd])
flaky_tests = fd.detect_flakes()

assert test_id in flaky_tests
assert flaky_tests[test_id] == {FlakeSymptomType.CONSECUTIVE_DIFF_OUTCOMES}


def test_flake_matching_failures_on_unrelated_branches(dbsession):
repoid = create_repo(
dbsession,
Expand Down
1 change: 0 additions & 1 deletion services/timeseries.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from database.models import Commit, Dataset, Measurement, MeasurementName
from database.models.core import Repository
from database.models.reports import RepositoryFlag
from database.models.timeseries import Dataset
from helpers.timeseries import backfill_max_batch_size, timeseries_enabled
from services.report import ReportService
from services.yaml import get_repo_yaml
Expand Down
12 changes: 0 additions & 12 deletions tasks/tests/unit/test_new_user_activated.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,18 +118,6 @@ def test_org_not_on_pr_plan(self, mocker, dbsession, pull):
"reason": "org not on pr author billing plan",
}

def test_org_not_on_pr_plan(self, mocker, dbsession, pull):
pull.repository.owner.plan = "users-inappm"
dbsession.flush()
res = NewUserActivatedTask().run_impl(
dbsession, pull.repository.owner.ownerid, pull.author.ownerid
)
assert res == {
"notifies_scheduled": False,
"pulls_notified": [],
"reason": "org not on pr author billing plan",
}

def test_no_commit_notifications_found(self, mocker, dbsession, pull):
mocked_possibly_resend_notifications = mocker.patch(
"tasks.new_user_activated.NewUserActivatedTask.possibly_resend_notifications"
Expand Down
2 changes: 1 addition & 1 deletion tasks/tests/unit/test_notify_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ def test__possibly_refresh_previous_selection(
assert task._possibly_refresh_previous_selection(commit) == True
mock_set_gh_app_for_commit.assert_called_with(app_to_save, commit)

def test__possibly_refresh_previous_selection(self, mocker, dbsession):
def test__possibly_refresh_previous_selection_false(self, mocker, dbsession):
commit = CommitFactory(repository__owner__service="github")
dbsession.add(commit)
dbsession.flush()
Expand Down
2 changes: 1 addition & 1 deletion tasks/tests/unit/test_sync_repo_languages.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def test_languages_intersection_and_synced_beyond_threshold(
task.run_impl(dbsession, repoid=repo.repoid, manual_trigger=False) == None
)

def test_languages_intersection_and_synced_beyond_threshold(
def test_languages_intersection_and_synced_beyond_threshold_with_languages(
self, dbsession, setup_with_languages
):
mocked_beyond_threshold = MOCKED_NOW + timedelta(days=-10)
Expand Down
1 change: 0 additions & 1 deletion tasks/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
from services.archive import ArchiveService
from services.bundle_analysis import BundleAnalysisReportService
from services.redis import (
Redis,
download_archive_from_redis,
get_parallel_upload_processing_session_counter_redis_key,
get_redis_connection,
Expand Down
Loading