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

Parallelize integration tests #411

Merged
merged 21 commits into from
Mar 27, 2024
Merged

Parallelize integration tests #411

merged 21 commits into from
Mar 27, 2024

Conversation

clavedeluna
Copy link
Contributor

@clavedeluna clavedeluna commented Mar 25, 2024

Overview

Be able to run all integration-tests in parallel for speed

Description

  • As we add more codemods, running integration tests in parallel is really important as these take a long time one after the other.
  • To do this, we had to move away from files in disk to testing with temp files /dirs. I was able to do this for all integration tests except sonar tests, which still require files in tests/samples because those align with sonar_issues.json. Later on hopefullly we can revisit the sonar issue here.
  • So for non-sonar integration tests, all relevant files (code, output file, dependency file) are temp files. For sonar the output file is a tempfile too.
  • As part of this work I also cleaned up quite a bit. One of my favorite things was making the replacement_lines be 1-index as this was confusing to see compared with expected_line_change. It didn't make sense to have these lines be 0-index when everything else for line changes is 1-indexed.
  • results: locally integration tests ran for me in 30s. In actions it takes <5min bc only 4 workers are used.

Additional Details

  • As a result of this work I discovered that one of the most recently added sonar integration tests test_sonar_fix_missing_self_or_cls is missing the is_node_selected check, it passes when it shouldn't. This is because I hadn't updated sonar_issues.json because nothing reminded me to do so. I'll ticket this and thoughts around how to prevent this issue.
  • We also want to figure out a better way to handle updating sonar_issues.json
  • For some reason unit test coverage lowered by 0.05 or so only in github actions (locally it's still > 94) so I just lowered ithe cutoff slightly.

class TestDependencyManager(CleanRepoMixin):
output_path = "test-codetf.txt"
class TestDependencyManager:
output_path = tempfile.mkstemp()[1]
Copy link
Member

Choose a reason for hiding this comment

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

Would it make more sense to do this within setup_class?


cls.code_path = os.path.join(cls.code_dir, cls.code_filename)

if cls.code_filename == "settings.py" and "Django" in str(cls):
Copy link
Member

Choose a reason for hiding this comment

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

It feels like maybe this should just happen in a Django-specific subclass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

very very true. Honestly I was just trying to make as few changes to specific integration test files as possible. Just for now I'm going to leave it as is.

Copy link

sonarcloud bot commented Mar 27, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@clavedeluna clavedeluna added this pull request to the merge queue Mar 27, 2024
Merged via the queue into main with commit 53ed01f Mar 27, 2024
12 checks passed
@clavedeluna clavedeluna deleted the int-test-faster branch March 27, 2024 11:20
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.

2 participants