Remove all "labels"-related code #782

Swatinem · 2024-10-14T11:42:42Z

The concepts of labels, labels_index and datapoints are all related to the defunct ATS (automatic test selection) product which was never adopted.

This PR starts out by removing the label_index feature, and then progressively removes all labels related code, along with never writing out new datapoints.

It turns out this feature flag was never fully rolled out, and is part of ATS (automated test selection) which was also not fully launched. This will now remove this feature flag, as a precursor to removing labels altogether.

codecov · 2024-10-14T11:46:05Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.98%. Comparing base (2973343) to head (78671e3).

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #782      +/-   ##
==========================================
- Coverage   98.03%   97.98%   -0.05%     
==========================================
  Files         440      431       -9     
  Lines       36566    34955    -1611     
==========================================
- Hits        35846    34252    -1594     
+ Misses        720      703      -17

Flag	Coverage Δ
integration	`97.98% <100.00%> (-0.05%)`	⬇️
unit	`97.98% <100.00%> (-0.05%)`	⬇️

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

Components	Coverage Δ
NonTestCode	`95.84% <100.00%> (-0.07%)`	⬇️
OutsideTasks	`97.99% <100.00%> (-0.02%)`	⬇️

Files with missing lines	Coverage Δ
rollouts/__init__.py	`100.00% <ø> (ø)`
services/report/__init__.py	`97.19% <100.00%> (-0.01%)`	⬇️
services/report/languages/pycoverage.py	`95.23% <100.00%> (-2.95%)`	⬇️
services/report/languages/tests/unit/__init__.py	`100.00% <ø> (ø)`
...ces/report/languages/tests/unit/test_pycoverage.py	`100.00% <ø> (ø)`
services/report/raw_upload_processor.py	`100.00% <100.00%> (+4.24%)`	⬆️
services/report/report_builder.py	`98.07% <100.00%> (-0.06%)`	⬇️
services/report/tests/unit/test_process.py	`99.06% <100.00%> (-0.01%)`	⬇️
services/report/tests/unit/test_report_builder.py	`100.00% <100.00%> (ø)`
services/report/tests/unit/test_sessions.py	`100.00% <100.00%> (ø)`
... and 6 more

... and 2 files with indirect coverage changes

codecov-staging · 2024-10-14T11:46:06Z

❌ 2 Tests Failed:

Tests completed	Failed	Passed	Skipped
1608	2	1606	0

View the top 2 failed tests by shortest run time

services.report.tests.unit.test_process.TestProcessRawUploadCarryforwardFlags test_process_raw_upload_with_carryforwarded_flags

Stack Traces | 0.032s run time

self = &lt;test_process.TestProcessRawUploadCarryforwardFlags object at 0x7fe99f0ef500&gt;

    def test_process_raw_upload_with_carryforwarded_flags(self):
        original_report = self._generate_sample_report()
        upload_flags = ["somethingold", "flag_two"]
        raw_content = dumps(
            {
                "coverage": {
                    "another.c": [None, 3, "1/2"],
                    "tests/test.py": [None, 0],
                    "folder/file.py": [None, 1],
                }
            }
        )
        uploaded_reports = LegacyParsedRawReport(
            toc=None,
            env=None,
            report_fixes=None,
            uploaded_files=[
                ParsedUploadedReportFile(
                    filename=".../path/to/app.coverage.json",
                    file_contents=BytesIO(raw_content.encode()),
                ),
            ],
        )
        session = Session(flags=upload_flags)
        result = process.process_raw_upload(
            UserYaml(
                {
                    "flag_management": {
                        "individual_flags": [
                            {"name": "somethingold", "carryforward": True}
                        ]
                    }
                }
            ),
            original_report,
            uploaded_reports,
            ["somethingold", "flag_two"],
            session=session,
        )
        report = result.report
&gt;       assert result.session_adjustment.fully_deleted_sessions == [1]
E       AttributeError: 'UploadProcessingResult' object has no attribute 'session_adjustment'

.../tests/unit/test_process.py:1112: AttributeError

services.tests.test_report.TestReportService test_get_existing_report_for_commit_already_carriedforward_add_sessions

Stack Traces | 0.052s run time

self = &lt;worker.services.tests.test_report.TestReportService object at 0x7fe99e697ec0&gt;
dbsession = &lt;sqlalchemy.orm.session.Session object at 0x7fe9887e23f0&gt;
sample_commit_with_report_big_already_carriedforward = Commit&lt;28fb6fad178e650bdc4eefef022f241223b92d54@repo&lt;524&gt;&gt;

    def test_get_existing_report_for_commit_already_carriedforward_add_sessions(
        self, dbsession, sample_commit_with_report_big_already_carriedforward
    ):
        commit = sample_commit_with_report_big_already_carriedforward
        dbsession.add(commit)
        dbsession.flush()
        yaml_dict = {"flags": {"enterprise": {"carryforward": True}}}
        report = ReportService(UserYaml(yaml_dict)).get_existing_report_for_commit(
            commit
        )
        assert report is not None
        assert len(report.files) == 15
        assert sorted(report.sessions.keys()) == [0, 1, 2, 3]
        first_to_merge_session = Session(flags=["enterprise"])
        report.add_session(first_to_merge_session)
        assert sorted(report.sessions.keys()) == [0, 1, 2, 3, 4]
        assert clear_carryforward_sessions(
            report, ["enterprise"], UserYaml(yaml_dict)
        ) == {2, 3}
        assert sorted(report.sessions.keys()) == [0, 1, 4]
        readable_report = self.convert_report_to_better_readable(report)
        expected_sessions_dict = {
            "0": {
                "N": None,
                "a": None,
                "c": None,
                "d": None,
                "e": None,
                "f": None,
                "j": None,
                "n": None,
                "p": None,
                "st": "uploaded",
                "se": {},
                "t": None,
                "u": None,
            },
            "1": {
                "N": None,
                "a": None,
                "c": None,
                "d": None,
                "e": None,
                "f": ["unit"],
                "j": None,
                "n": None,
                "p": None,
                "st": "uploaded",
                "se": {},
                "t": None,
                "u": None,
            },
            "4": {
                "N": None,
                "a": None,
                "c": None,
                "d": None,
                "e": None,
                "f": ["enterprise"],
                "j": None,
                "n": None,
                "p": None,
                "st": "uploaded",
                "se": {},
                "t": None,
                "u": None,
            },
        }
        assert readable_report["report"]["sessions"]["0"] == expected_sessions_dict["0"]
        assert readable_report["report"]["sessions"]["1"] == expected_sessions_dict["1"]
        assert readable_report["report"]["sessions"]["4"] == expected_sessions_dict["4"]
        assert readable_report["report"]["sessions"] == expected_sessions_dict
        newly_added_session = {
            "N": None,
            "a": None,
            "c": None,
            "d": None,
            "e": None,
            "f": ["unit"],
            "j": None,
            "n": None,
            "p": None,
            "st": "uploaded",
            "se": {},
            "t": None,
            "u": None,
        }
        second_to_merge_session = Session(flags=["unit"])
        report.add_session(second_to_merge_session)
        assert sorted(report.sessions.keys()) == [0, 1, 3, 4]
&gt;       assert clear_carryforward_sessions(report, ["unit"], UserYaml(yaml_dict)) == {}
E       assert set() == {}
E         
E         Use -v to get more diff

services/tests/test_report.py:1566: AssertionError

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

codecov-qa · 2024-10-14T11:46:17Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.98%. Comparing base (2973343) to head (78671e3).

✅ All tests successful. No failed tests found.

@@            Coverage Diff             @@
##             main     #782      +/-   ##
==========================================
- Coverage   98.03%   97.98%   -0.05%     
==========================================
  Files         440      431       -9     
  Lines       36566    34955    -1611     
==========================================
- Hits        35846    34252    -1594     
+ Misses        720      703      -17

Flag	Coverage Δ
integration	`97.98% <100.00%> (-0.05%)`	⬇️
unit	`97.98% <100.00%> (-0.05%)`	⬇️

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

Components	Coverage Δ
NonTestCode	`95.84% <100.00%> (-0.07%)`	⬇️
OutsideTasks	`97.99% <100.00%> (-0.02%)`	⬇️

Files with missing lines	Coverage Δ
rollouts/__init__.py	`100.00% <ø> (ø)`
services/report/__init__.py	`97.19% <100.00%> (-0.01%)`	⬇️
services/report/languages/pycoverage.py	`95.23% <100.00%> (-2.95%)`	⬇️
services/report/languages/tests/unit/__init__.py	`100.00% <ø> (ø)`
...ces/report/languages/tests/unit/test_pycoverage.py	`100.00% <ø> (ø)`
services/report/raw_upload_processor.py	`100.00% <100.00%> (+4.24%)`	⬆️
services/report/report_builder.py	`98.07% <100.00%> (-0.06%)`	⬇️
services/report/tests/unit/test_process.py	`99.06% <100.00%> (-0.01%)`	⬇️
services/report/tests/unit/test_report_builder.py	`100.00% <100.00%> (ø)`
services/report/tests/unit/test_sessions.py	`100.00% <100.00%> (ø)`
... and 6 more

... and 2 files with indirect coverage changes

codecov-public-qa · 2024-10-14T11:46:22Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.98%. Comparing base (2973343) to head (78671e3).

✅ All tests successful. No failed tests found.

@@            Coverage Diff             @@
##             main     #782      +/-   ##
==========================================
- Coverage   98.03%   97.98%   -0.05%     
==========================================
  Files         440      431       -9     
  Lines       36566    34955    -1611     
==========================================
- Hits        35846    34252    -1594     
+ Misses        720      703      -17

Flag	Coverage Δ
integration	`97.98% <100.00%> (-0.05%)`	⬇️
unit	`97.98% <100.00%> (-0.05%)`	⬇️

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

Components	Coverage Δ
NonTestCode	`95.84% <100.00%> (-0.07%)`	⬇️
OutsideTasks	`97.99% <100.00%> (-0.02%)`	⬇️

Files	Coverage Δ
rollouts/__init__.py	`100.00% <ø> (ø)`
services/report/__init__.py	`97.19% <100.00%> (-0.01%)`	⬇️
services/report/languages/pycoverage.py	`95.23% <100.00%> (-2.95%)`	⬇️
services/report/languages/tests/unit/__init__.py	`100.00% <ø> (ø)`
...ces/report/languages/tests/unit/test_pycoverage.py	`100.00% <ø> (ø)`
services/report/raw_upload_processor.py	`100.00% <100.00%> (+4.24%)`	⬆️
services/report/report_builder.py	`98.07% <100.00%> (-0.06%)`	⬇️
services/report/tests/unit/test_process.py	`99.06% <100.00%> (-0.01%)`	⬇️
services/report/tests/unit/test_report_builder.py	`100.00% <100.00%> (ø)`
services/report/tests/unit/test_sessions.py	`100.00% <100.00%> (ø)`
... and 6 more

... and 2 files with indirect coverage changes

giovanni-guidini

LGTM

Swatinem added 3 commits October 14, 2024 13:07

Remove label_index feature flag

fc29e8f

It turns out this feature flag was never fully rolled out, and is part of ATS (automated test selection) which was also not fully launched. This will now remove this feature flag, as a precursor to removing labels altogether.

Remove most of the "labels" concept

c7b8873

rip out more of labels

a862064

Swatinem requested a review from a team October 14, 2024 11:42

Swatinem self-assigned this Oct 14, 2024

Swatinem added 2 commits October 14, 2024 13:54

unregister removed tasks

f2b8e07

fix tests and more code removal related to partial sessions

78671e3

Swatinem force-pushed the swatinem/rm-labels branch from 63c6a46 to 78671e3 Compare October 14, 2024 12:34

Swatinem marked this pull request as ready for review October 14, 2024 12:53

giovanni-guidini approved these changes Oct 14, 2024

View reviewed changes

Swatinem mentioned this pull request Oct 14, 2024

Remove existing ATS / "labels"-related code codecov/engineering-team#2672

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove all "labels"-related code #782

Remove all "labels"-related code #782

Swatinem commented Oct 14, 2024

codecov bot commented Oct 14, 2024 •

edited

Loading

codecov-staging bot commented Oct 14, 2024 •

edited

Loading

codecov-qa bot commented Oct 14, 2024 •

edited

Loading

codecov-public-qa bot commented Oct 14, 2024 •

edited

Loading

giovanni-guidini left a comment

Remove all "labels"-related code #782

Are you sure you want to change the base?

Remove all "labels"-related code #782

Conversation

Swatinem commented Oct 14, 2024

codecov bot commented Oct 14, 2024 • edited Loading

Codecov Report

codecov-staging bot commented Oct 14, 2024 • edited Loading

❌ 2 Tests Failed:

codecov-qa bot commented Oct 14, 2024 • edited Loading

Codecov Report

codecov-public-qa bot commented Oct 14, 2024 • edited Loading

Codecov Report

giovanni-guidini left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 14, 2024 •

edited

Loading

codecov-staging bot commented Oct 14, 2024 •

edited

Loading

codecov-qa bot commented Oct 14, 2024 •

edited

Loading

codecov-public-qa bot commented Oct 14, 2024 •

edited

Loading