-
Notifications
You must be signed in to change notification settings - Fork 22
[FIX] Correctly resolve test files paths when adding runtime comments #831
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] Correctly resolve test files paths when adding runtime comments #831
Conversation
|
Codeflash Bot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
| ) | ||
| abs_path = str(Path(inv_id.test_module_path.replace(".", os.sep)).with_suffix(".py").resolve().with_suffix("")) | ||
| abs_path = tests_project_rootdir / Path(inv_id.test_module_path.replace(".", os.sep)).with_suffix(".py") | ||
| abs_path = str(abs_path.resolve().with_suffix("")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might want to test with different test dirs, some which are a subdirectory of the module root some which are not. I remember i was using the test_dir early on but it didn't work for all scenarioes @mohammedahmed18
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aseembits93
tested with a tests dir inside the module root, and the runtime diff is shown correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
outside the module root? @mohammedahmed18
|
@mohammedahmed18 branch conflicts after pygls migration |
…rect-resolve-test-paths-for-runtime-comments
…rect-resolve-test-paths-for-runtime-comments
… github.com:codeflash-ai/codeflash into fix/correct-resolve-test-paths-for-runtime-comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
async e2e test doesn't show it yet, let me investigate
|
|
|
@KRRT7 |
|
ok the mutation is expected, it's somehow working as expected downstream for everything except async mode. somehow the async mode invocation ids have a different test_module_path (should be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works for non-async. async issue is unrelated.
|
@aseembits93 found the issue, the tests_project_rootdir is being overridden during discovering tests if pytest_rootdir is not None:
cfg.tests_project_rootdir = Path(pytest_rootdir)EDIT: this is weird it really does work with the non-async code even with tests_project_rootdir being overriden |
PR Type
Bug fix, Enhancement
this will fix the bug where runtime comments not shown in worktree mode

Description
Correct test path resolution for comments
Pass tests rootdir into runtime mapping
Simplify optimizer file/path initialization
Diagram Walkthrough
File Walkthrough
edit_generated_tests.py
Resolve test paths using provided tests rootdircodeflash/code_utils/edit_generated_tests.py
beta.py
Simplify optimizer path setup in LSP initcodeflash/lsp/beta.py
function_optimizer.py
Provide tests rootdir to comment annotatorcodeflash/optimization/function_optimizer.py
add_runtime_comments_to_generated_tests.