Skip to content

Commit

Permalink
fix: dont save report in processor and add failure_message to testrun…
Browse files Browse the repository at this point in the history 
…_list

Going to move the save report part to the finisher to avoid concurrency issues
  • Loading branch information
@joseph-sentry
joseph-sentry committed Jan 11, 2024
1 parent f93d2bf commit d522306
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 161 deletions.
60 changes: 6 additions & 54 deletions tasks/test_results_processor.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,10 @@ async def run_async(

upload_list.append(upload_obj)

# save relevant stuff to database
self.save_report(db_session, testrun_list, upload_obj)
testrun_dict_list = []

for test in testrun_list:
testrun_dict_list.append(self.testrun_to_dict(test))

# kick off notification task stuff

Expand All @@ -103,60 +105,9 @@ async def run_async(

return {
"successful": True,
"testrun_list": [self.testrun_to_dict(t) for t in testrun_list],
"testrun_list": testrun_dict_list,
}

def save_report(
self, db_session: Session, testrun_list: List[Testrun], upload: Upload
):
repo_tests = (
db_session.query(Test).filter_by(repoid=upload.report.commit.repoid).all()
)
# Issue here is that the test result processing tasks are running in parallel
# The idea is that we can first get a list of existing tests from the database
# if a test is not found in that list we try to insert it has already inserted the test
# so we should just fetch it

# however, this may cause significant performance issues the first time a user runs test
# result ingestion on a large project

test_dict = dict()
for test in repo_tests:
test_dict[f"{test.testsuite}::{test.name}"] = test
for testrun in testrun_list:
test = test_dict.get(f"{testrun.testsuite}::{testrun.name}", None)
if not test:
try:
test = Test(
repoid=upload.report.commit.repoid,
name=testrun.name,
testsuite=testrun.testsuite,
)
db_session.add(test)
db_session.commit()
except IntegrityError:
db_session.rollback()
test = (
db_session.query(Test)
.filter_by(
repoid=upload.report.commit.repoid,
name=testrun.name,
testsuite=testrun.testsuite,
)
.first()
)

db_session.add(
TestInstance(
test_id=test.id,
duration_seconds=testrun.duration,
outcome=int(testrun.outcome),
upload_id=upload.id,
failure_message=testrun.failure_message,
)
)
db_session.flush()

def process_individual_arg(self, upload: Upload, repository) -> List[Testrun]:
archive_service = ArchiveService(repository)

Expand Down Expand Up @@ -227,6 +178,7 @@ def testrun_to_dict(self, t: Testrun):
"name": t.name,
"testsuite": t.testsuite,
"duration_seconds": t.duration,
"failure_message": t.failure_message,
}

def should_delete_archive(self, commit_yaml):
Expand Down
121 changes: 14 additions & 107 deletions tasks/tests/unit/test_test_results_processor_task.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,24 +63,28 @@ async def test_upload_processor_task_call(
"name": "api.temp.calculator.test_calculator::test_add",
"outcome": "Outcome.Pass",
"testsuite": "pytest",
"failure_message": None,
},
{
"duration_seconds": 0.001,
"name": "api.temp.calculator.test_calculator::test_subtract",
"outcome": "Outcome.Pass",
"testsuite": "pytest",
"failure_message": None,
},
{
"duration_seconds": 0.0,
"name": "api.temp.calculator.test_calculator::test_multiply",
"outcome": "Outcome.Pass",
"testsuite": "pytest",
"failure_message": None,
},
{
"duration_seconds": 0.001,
"name": "api.temp.calculator.test_calculator::test_divide",
"outcome": "Outcome.Failure",
"testsuite": "pytest",
"failure_message": "def test_divide():\n> assert Calculator.divide(1, 2) == 0.5\nE assert 1.0 == 0.5\nE + where 1.0 = <function Calculator.divide at 0x104c9eb90>(1, 2)\nE + where <function Calculator.divide at 0x104c9eb90> = Calculator.divide\n\napi/temp/calculator/test_calculator.py:30: AssertionError",
},
],
}
Expand Down Expand Up @@ -137,12 +141,14 @@ async def test_upload_processor_task_call_pytest_reportlog(
"name": "TestParsers.test_junit[./tests/junit.xml-expected0]",
"outcome": "Outcome.Pass",
"testsuite": "tests/test_parsers.py",
"failure_message": None,
},
{
"duration_seconds": 0.0008068084716796875,
"name": "TestParsers.test_junit[./tests/jest-junit.xml-expected1]",
"outcome": "Outcome.Pass",
"testsuite": "tests/test_parsers.py",
"failure_message": None,
},
],
}
Expand Down Expand Up @@ -199,24 +205,28 @@ async def test_upload_processor_task_call_vitest(
"name": " first test file 2 + 2 should equal 4",
"outcome": "Outcome.Failure",
"testsuite": "/root-directory/__tests__/test-file-1.test.ts",
"failure_message": "expected 5 to be 4 // Object.is equality",
},
{
"duration_seconds": 0.009,
"name": " first test file 2 + 2 should equal 4",
"outcome": "Outcome.Failure",
"testsuite": "/root-directory/__tests__/test-file-1.test.ts",
"failure_message": "expected 5 to be 4 // Object.is equality",
},
{
"duration_seconds": 0.009,
"name": " first test file 2 + 2 should equal 4",
"outcome": "Outcome.Failure",
"testsuite": "/root-directory/__tests__/test-file-1.test.ts",
"failure_message": "expected 5 to be 4 // Object.is equality",
},
{
"duration_seconds": 0.009,
"name": " first test file 2 + 2 should equal 4",
"outcome": "Outcome.Failure",
"testsuite": "/root-directory/__tests__/test-file-1.test.ts",
"failure_message": "expected 5 to be 4 // Object.is equality",
},
],
}
Expand Down Expand Up @@ -332,24 +342,28 @@ async def test_test_result_processor_task_delete_archive(
"name": " first test file 2 + 2 should equal 4",
"outcome": "Outcome.Failure",
"testsuite": "/root-directory/__tests__/test-file-1.test.ts",
"failure_message": "expected 5 to be 4 // Object.is equality",
},
{
"duration_seconds": 0.009,
"name": " first test file 2 + 2 should equal 4",
"outcome": "Outcome.Failure",
"testsuite": "/root-directory/__tests__/test-file-1.test.ts",
"failure_message": "expected 5 to be 4 // Object.is equality",
},
{
"duration_seconds": 0.009,
"name": " first test file 2 + 2 should equal 4",
"outcome": "Outcome.Failure",
"testsuite": "/root-directory/__tests__/test-file-1.test.ts",
"failure_message": "expected 5 to be 4 // Object.is equality",
},
{
"duration_seconds": 0.009,
"name": " first test file 2 + 2 should equal 4",
"outcome": "Outcome.Failure",
"testsuite": "/root-directory/__tests__/test-file-1.test.ts",
"failure_message": "expected 5 to be 4 // Object.is equality",
},
],
}
Expand Down Expand Up @@ -409,110 +423,3 @@ async def test_test_result_processor_task_bad_file(

assert expected_result == result
assert "File did not match any parser format" in caplog.text

@pytest.mark.asyncio
@pytest.mark.integration
async def test_upload_processor_task_call_multiple(
self,
mocker,
mock_configuration,
dbsession,
codecov_vcr,
mock_storage,
mock_redis,
celery_app,
):
url = "v4/raw/2019-05-22/C3C4715CA57C910D11D5EB899FC86A7E/4c4e4654ac25037ae869caeb3619d485970b6304/a84d445c-9c1e-434f-8275-f18f1f320f81.txt"
with open(here.parent.parent / "samples" / "sample_test.txt") as f:
content = f.read()
mock_storage.write_file("archive", url, content)
upload = UploadFactory.create(storage_path=url)
upload_2 = UploadFactory.create(storage_path=url)
dbsession.add(upload)
dbsession.flush()
redis_queue = [{"url": url, "upload_pk": upload.id_}]
mocker.patch.object(TestResultsProcessorTask, "app", celery_app)

commit = CommitFactory.create(
message="hello world",
commitid="cd76b0821854a780b60012aed85af0a8263004ad",
repository__owner__unencrypted_oauth_token="test7lk5ndmtqzxlx06rip65nac9c7epqopclnoy",
repository__owner__username="joseph-sentry",
repository__owner__service="github",
repository__name="codecov-demo",
)
commit_2 = CommitFactory.create(
message="hello world",
commitid="cd76b0821854a780b60012aed85af0a8263004ae",
)

commit_2.repository = commit.repository
dbsession.add(commit)
dbsession.add(commit_2)
dbsession.flush()
current_report_row = CommitReport(commit_id=commit.id_)
current_report_row_2 = CommitReport(commit_id=commit_2.id_)
dbsession.add(current_report_row)
dbsession.add(current_report_row_2)
dbsession.flush()
upload.report = current_report_row
upload_2.report = current_report_row_2

result = await TestResultsProcessorTask().run_async(
dbsession,
repoid=commit.repoid,
commitid=commit.commitid,
commit_yaml={"codecov": {"max_report_age": False}},
arguments_list=redis_queue,
)
expected_result = {
"successful": True,
"testrun_list": [
{
"duration_seconds": 0.001,
"name": "api.temp.calculator.test_calculator::test_add",
"outcome": "Outcome.Pass",
"testsuite": "pytest",
},
{
"duration_seconds": 0.001,
"name": "api.temp.calculator.test_calculator::test_subtract",
"outcome": "Outcome.Pass",
"testsuite": "pytest",
},
{
"duration_seconds": 0.0,
"name": "api.temp.calculator.test_calculator::test_multiply",
"outcome": "Outcome.Pass",
"testsuite": "pytest",
},
{
"duration_seconds": 0.001,
"name": "api.temp.calculator.test_calculator::test_divide",
"outcome": "Outcome.Failure",
"testsuite": "pytest",
},
],
}
assert expected_result == result
assert commit.message == "hello world"

url = "v4/raw/2019-05-22/C3C4715CA57C910D11D5EB899FC86A7E/4c4e4654ac25037ae869caeb3619d485970b6304/a84d445c-9c1e-434f-8275-f18f1f320f80.txt"
with open(here.parent.parent / "samples" / "sample_test.txt") as f:
content = f.read()
mock_storage.write_file("archive", url, content)
redis_queue = [{"url": url, "upload_pk": upload_2.id_}]

result = await TestResultsProcessorTask().run_async(
dbsession,
repoid=commit_2.repoid,
commitid=commit_2.commitid,
commit_yaml={"codecov": {"max_report_age": False}},
arguments_list=redis_queue,
)

repo_tests = (
dbsession.query(Test).filter_by(repoid=upload.report.commit.repoid).all()
)

assert len(repo_tests) == 4

0 comments on commit d522306

Please sign in to comment.