mirror of
https://github.com/qodo-ai/pr-agent.git
synced 2025-12-11 18:35:18 +00:00
Compare commits
6 commits
dd3a98adaa
...
285f47c6c4
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
285f47c6c4 | ||
|
|
bf5da9a9fb | ||
|
|
d3878ef412 | ||
|
|
979bc6788e | ||
|
|
4f5e1f708c | ||
|
|
1764be1e98 |
4 changed files with 427 additions and 6 deletions
|
|
@ -56,7 +56,7 @@ A `PR Code Verified` label indicates the PR code meets ticket requirements, but
|
|||
|
||||
-
|
||||
|
||||
By default, the tool will automatically validate if the PR complies with the referenced ticket.
|
||||
By default, the `review` tool will automatically validate if the PR complies with the referenced ticket.
|
||||
If you want to disable this feedback, add the following line to your configuration file:
|
||||
|
||||
```toml
|
||||
|
|
@ -75,6 +75,33 @@ A `PR Code Verified` label indicates the PR code meets ticket requirements, but
|
|||
|
||||
the `review` tool will also validate that the PR code doesn't contain any additional content that is not related to the ticket. If it does, the PR will be labeled at best as `PR Code Verified`, and the `review` tool will provide a comment with the additional unrelated content found in the PR code.
|
||||
|
||||
### Compliance tool
|
||||
|
||||
The `compliance` tool also uses ticket context to validate that PR changes fulfill the requirements specified in linked tickets.
|
||||
|
||||
#### Configuration options
|
||||
|
||||
-
|
||||
|
||||
By default, the `compliance` tool will automatically validate if the PR complies with the referenced ticket.
|
||||
If you want to disable ticket compliance checking in the compliance tool, add the following line to your configuration file:
|
||||
|
||||
```toml
|
||||
[pr_compliance]
|
||||
require_ticket_analysis_review=false
|
||||
```
|
||||
|
||||
-
|
||||
|
||||
If you set:
|
||||
```toml
|
||||
[pr_compliance]
|
||||
check_pr_additional_content=true
|
||||
```
|
||||
(default: `false`)
|
||||
|
||||
the `compliance` tool will also validate that the PR code doesn't contain any additional content that is not related to the ticket.
|
||||
|
||||
## GitHub/Gitlab Issues Integration
|
||||
|
||||
Qodo Merge will automatically recognize GitHub/Gitlab issues mentioned in the PR description and fetch the issue content.
|
||||
|
|
|
|||
|
|
@ -329,6 +329,10 @@ enable_global_pr_compliance = true
|
|||
???+ example "Ticket compliance options"
|
||||
|
||||
<table>
|
||||
<tr>
|
||||
<td><b>require_ticket_analysis_review</b></td>
|
||||
<td>If set to true, the tool will fetch and analyze ticket context for compliance validation. Default is true.</td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td><b>enable_ticket_labels</b></td>
|
||||
<td>If set to true, the tool will add ticket compliance labels to the PR. Default is false.</td>
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ def load(obj, env=None, silent=True, key=None, filename=None):
|
|||
- Replaces list and dict fields instead of appending/updating (non-default Dynaconf behavior).
|
||||
- Enforces several security checks (e.g., disallows includes/preloads and enforces .toml files).
|
||||
- Supports optional single-key loading.
|
||||
- Supports Dynaconf's fresh_vars feature for dynamic reloading.
|
||||
Args:
|
||||
obj: The Dynaconf settings instance to update.
|
||||
env: The current environment name (upper case). Defaults to 'DEVELOPMENT'. Note: currently unused.
|
||||
|
|
@ -93,7 +94,8 @@ def load(obj, env=None, silent=True, key=None, filename=None):
|
|||
|
||||
# Update the settings object
|
||||
for k, v in accumulated_data.items():
|
||||
if key is None or key == k:
|
||||
# For fresh_vars support: key parameter is uppercase, but accumulated_data keys are lowercase
|
||||
if key is None or key.upper() == k.upper():
|
||||
obj.set(k, v)
|
||||
|
||||
def validate_file_security(file_data, filename):
|
||||
|
|
|
|||
388
tests/unittest/test_fresh_vars_functionality.py
Normal file
388
tests/unittest/test_fresh_vars_functionality.py
Normal file
|
|
@ -0,0 +1,388 @@
|
|||
"""
|
||||
Comprehensive unit tests for Dynaconf fresh_vars functionality.
|
||||
|
||||
These tests verify that the fresh_vars feature works correctly with the custom_merge_loader,
|
||||
particularly for the GitLab credentials use case where values should be reloaded from disk
|
||||
on each access rather than being cached.
|
||||
|
||||
The tests are designed to detect if fresh_vars is broken due to custom loader changes,
|
||||
such as those introduced in https://github.com/qodo-ai/pr-agent/pull/2087.
|
||||
"""
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from dynaconf import Dynaconf
|
||||
|
||||
# Import get_settings at module level to complete the import chain and avoid circular import issues
|
||||
# This ensures pr_agent.config_loader is fully loaded before custom_merge_loader is used in tests
|
||||
from pr_agent.config_loader import get_settings # noqa: F401
|
||||
|
||||
|
||||
# Module-level helper function
|
||||
def create_dynaconf_with_custom_loader(temp_dir, secrets_file):
|
||||
"""
|
||||
Create a Dynaconf instance matching the production configuration.
|
||||
|
||||
This mimics the config_loader.py setup with:
|
||||
- core_loaders disabled
|
||||
- custom_merge_loader and env_loader enabled
|
||||
- merge_enabled = True
|
||||
|
||||
Note: fresh_vars should be configured via FRESH_VARS_FOR_DYNACONF environment variable,
|
||||
which is the only way to configure it in pr-agent.
|
||||
|
||||
Args:
|
||||
temp_dir: Temporary directory path
|
||||
secrets_file: Path to secrets file
|
||||
|
||||
Returns:
|
||||
Dynaconf instance configured like production
|
||||
"""
|
||||
return Dynaconf(
|
||||
core_loaders=[],
|
||||
loaders=["pr_agent.custom_merge_loader", "dynaconf.loaders.env_loader"],
|
||||
root_path=temp_dir,
|
||||
merge_enabled=True,
|
||||
envvar_prefix=False,
|
||||
load_dotenv=False,
|
||||
settings_files=[str(secrets_file)],
|
||||
)
|
||||
|
||||
|
||||
class TestFreshVarsGitLabScenario:
|
||||
"""
|
||||
Test fresh_vars functionality for the GitLab credentials use case.
|
||||
|
||||
This class tests the specific scenario where:
|
||||
- FRESH_VARS_FOR_DYNACONF='["GITLAB"]' is set
|
||||
- .secrets.toml contains gitlab.personal_access_token and gitlab.shared_secret
|
||||
- Values should be reloaded from disk on each access (not cached)
|
||||
"""
|
||||
|
||||
def setup_method(self):
|
||||
"""Set up temporary directory and files for each test."""
|
||||
self.temp_dir = tempfile.mkdtemp()
|
||||
self.secrets_file = Path(self.temp_dir) / ".secrets.toml"
|
||||
|
||||
def teardown_method(self):
|
||||
"""Clean up temporary files after each test."""
|
||||
import shutil
|
||||
|
||||
if hasattr(self, "temp_dir") and Path(self.temp_dir).exists():
|
||||
shutil.rmtree(self.temp_dir)
|
||||
|
||||
def create_secrets_toml(self, personal_access_token="initial_token", shared_secret="initial_secret"):
|
||||
"""
|
||||
Create a .secrets.toml file with GitLab credentials.
|
||||
|
||||
Args:
|
||||
personal_access_token: The GitLab personal access token value
|
||||
shared_secret: The GitLab shared secret value
|
||||
"""
|
||||
content = f"""[gitlab]
|
||||
personal_access_token = "{personal_access_token}"
|
||||
shared_secret = "{shared_secret}"
|
||||
"""
|
||||
self.secrets_file.write_text(content)
|
||||
|
||||
def test_gitlab_personal_access_token_reload(self):
|
||||
"""
|
||||
Test that gitlab.personal_access_token is reloaded when marked as fresh.
|
||||
|
||||
This is the critical test for the user's use case. It verifies that:
|
||||
1. Initial value is loaded correctly
|
||||
2. After modifying the file, the new value is returned (not cached)
|
||||
3. This works with the custom_merge_loader
|
||||
"""
|
||||
# Create initial secrets file
|
||||
self.create_secrets_toml(personal_access_token="token_v1", shared_secret="secret_v1")
|
||||
|
||||
# Set FRESH_VARS_FOR_DYNACONF environment variable (the only way to configure fresh_vars in pr-agent)
|
||||
with patch.dict(os.environ, {"FRESH_VARS_FOR_DYNACONF": '["GITLAB"]'}):
|
||||
# Create Dynaconf with GITLAB marked as fresh via env var
|
||||
settings = create_dynaconf_with_custom_loader(self.temp_dir, self.secrets_file)
|
||||
|
||||
# First access - should return initial value
|
||||
first_token = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
assert first_token == "token_v1", "Initial personal_access_token should be 'token_v1'"
|
||||
|
||||
# Modify the secrets file
|
||||
self.create_secrets_toml(personal_access_token="token_v2_updated", shared_secret="secret_v1")
|
||||
|
||||
# Second access - should return NEW value (not cached)
|
||||
second_token = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
assert second_token == "token_v2_updated", (
|
||||
"After file modification, personal_access_token should be reloaded to 'token_v2_updated'"
|
||||
)
|
||||
|
||||
# Verify the values are different (fresh_vars working)
|
||||
assert first_token != second_token, "fresh_vars should cause values to be reloaded, not cached"
|
||||
|
||||
def test_gitlab_multiple_fields_reload(self):
|
||||
"""
|
||||
Test that both gitlab fields reload together when GITLAB is marked as fresh.
|
||||
|
||||
This verifies that fresh_vars works correctly when multiple fields
|
||||
in the same section are modified simultaneously.
|
||||
"""
|
||||
# Create initial secrets file
|
||||
self.create_secrets_toml(personal_access_token="token_v1", shared_secret="secret_v1")
|
||||
|
||||
# Set FRESH_VARS_FOR_DYNACONF environment variable
|
||||
with patch.dict(os.environ, {"FRESH_VARS_FOR_DYNACONF": '["GITLAB"]'}):
|
||||
# Create Dynaconf with GITLAB marked as fresh via env var
|
||||
settings = create_dynaconf_with_custom_loader(self.temp_dir, self.secrets_file)
|
||||
|
||||
# First access - both fields
|
||||
first_token = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
first_secret = settings.GITLAB.SHARED_SECRET
|
||||
assert first_token == "token_v1"
|
||||
assert first_secret == "secret_v1"
|
||||
|
||||
# Modify both fields in the secrets file
|
||||
self.create_secrets_toml(
|
||||
personal_access_token="token_v2_both_updated", shared_secret="secret_v2_both_updated"
|
||||
)
|
||||
|
||||
# Second access - both fields should be updated
|
||||
second_token = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
second_secret = settings.GITLAB.SHARED_SECRET
|
||||
|
||||
assert second_token == "token_v2_both_updated", "personal_access_token should be reloaded"
|
||||
assert second_secret == "secret_v2_both_updated", "shared_secret should be reloaded"
|
||||
|
||||
# Verify both fields were reloaded
|
||||
assert first_token != second_token, "personal_access_token should not be cached"
|
||||
assert first_secret != second_secret, "shared_secret should not be cached"
|
||||
|
||||
|
||||
class TestFreshVarsCustomLoaderIntegration:
|
||||
"""
|
||||
Test fresh_vars integration with custom_merge_loader.
|
||||
|
||||
These tests verify that fresh_vars works correctly when using the
|
||||
custom_merge_loader instead of Dynaconf's default core loaders.
|
||||
"""
|
||||
|
||||
def setup_method(self):
|
||||
"""Set up temporary directory and files for each test."""
|
||||
self.temp_dir = tempfile.mkdtemp()
|
||||
self.secrets_file = Path(self.temp_dir) / ".secrets.toml"
|
||||
|
||||
def teardown_method(self):
|
||||
"""Clean up temporary files after each test."""
|
||||
import shutil
|
||||
|
||||
if hasattr(self, "temp_dir") and Path(self.temp_dir).exists():
|
||||
shutil.rmtree(self.temp_dir)
|
||||
|
||||
def create_secrets_toml(self, personal_access_token="initial_token", shared_secret="initial_secret"):
|
||||
"""Create a .secrets.toml file with GitLab credentials."""
|
||||
content = f"""[gitlab]
|
||||
personal_access_token = "{personal_access_token}"
|
||||
shared_secret = "{shared_secret}"
|
||||
"""
|
||||
self.secrets_file.write_text(content)
|
||||
|
||||
def test_fresh_vars_without_core_loaders(self):
|
||||
"""
|
||||
Critical test: Verify fresh_vars works when core_loaders are disabled.
|
||||
|
||||
This test detects if the bug exists where fresh_vars stops working
|
||||
when core_loaders=[] is set. This is the key issue that may have been
|
||||
introduced by the custom_merge_loader changes.
|
||||
|
||||
Expected behavior:
|
||||
- If fresh_vars works: second_value != first_value
|
||||
- If fresh_vars is broken: second_value == first_value (cached)
|
||||
"""
|
||||
# Create initial secrets file
|
||||
self.create_secrets_toml(personal_access_token="token_before_bug_test")
|
||||
|
||||
# Set FRESH_VARS_FOR_DYNACONF environment variable
|
||||
with patch.dict(os.environ, {"FRESH_VARS_FOR_DYNACONF": '["GITLAB"]'}):
|
||||
# Create Dynaconf WITHOUT core loaders but WITH fresh_vars via env var
|
||||
settings = create_dynaconf_with_custom_loader(self.temp_dir, self.secrets_file)
|
||||
|
||||
# First access
|
||||
first_value = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
assert first_value == "token_before_bug_test", "Initial value should be loaded correctly"
|
||||
|
||||
# Modify the file
|
||||
self.create_secrets_toml(personal_access_token="token_after_bug_test")
|
||||
|
||||
# Second access - THIS IS THE CRITICAL CHECK
|
||||
second_value = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
|
||||
# If this assertion fails, fresh_vars is broken with custom_merge_loader
|
||||
assert second_value == "token_after_bug_test", (
|
||||
"CRITICAL: fresh_vars should reload the value even with core_loaders=[]"
|
||||
)
|
||||
|
||||
assert first_value != second_value, "CRITICAL: Values should be different, indicating fresh_vars is working"
|
||||
|
||||
def test_custom_loader_respects_fresh_vars(self):
|
||||
"""
|
||||
Test that custom_merge_loader respects the fresh_vars configuration.
|
||||
|
||||
Verifies that when a section is marked as fresh, the custom loader
|
||||
doesn't cache values from that section.
|
||||
"""
|
||||
# Create initial secrets file with multiple sections
|
||||
content = """[gitlab]
|
||||
personal_access_token = "gitlab_token_v1"
|
||||
|
||||
[github]
|
||||
user_token = "github_token_v1"
|
||||
"""
|
||||
self.secrets_file.write_text(content)
|
||||
|
||||
# Set FRESH_VARS_FOR_DYNACONF environment variable (only GITLAB)
|
||||
with patch.dict(os.environ, {"FRESH_VARS_FOR_DYNACONF": '["GITLAB"]'}):
|
||||
# Create Dynaconf with only GITLAB marked as fresh via env var
|
||||
settings = create_dynaconf_with_custom_loader(self.temp_dir, self.secrets_file)
|
||||
|
||||
# Access both sections
|
||||
gitlab_token_1 = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
github_token_1 = settings.GITHUB.USER_TOKEN
|
||||
|
||||
# Modify both sections
|
||||
content = """[gitlab]
|
||||
personal_access_token = "gitlab_token_v2"
|
||||
|
||||
[github]
|
||||
user_token = "github_token_v2"
|
||||
"""
|
||||
self.secrets_file.write_text(content)
|
||||
|
||||
# Access again
|
||||
gitlab_token_2 = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
github_token_2 = settings.GITHUB.USER_TOKEN
|
||||
|
||||
# GITLAB should be reloaded (marked as fresh)
|
||||
assert gitlab_token_2 == "gitlab_token_v2", "GITLAB section should be reloaded (marked as fresh)"
|
||||
assert gitlab_token_1 != gitlab_token_2, "GITLAB values should not be cached"
|
||||
|
||||
# GITHUB should be cached (not marked as fresh)
|
||||
assert github_token_2 == "github_token_v1", "GITHUB section should be cached (not marked as fresh)"
|
||||
assert github_token_1 == github_token_2, "GITHUB values should be cached"
|
||||
|
||||
|
||||
class TestFreshVarsBasicFunctionality:
|
||||
"""
|
||||
Test basic fresh_vars functionality and edge cases.
|
||||
|
||||
These tests verify fundamental fresh_vars behavior and ensure
|
||||
the feature works as expected in various scenarios.
|
||||
"""
|
||||
|
||||
def setup_method(self):
|
||||
"""Set up temporary directory and files for each test."""
|
||||
self.temp_dir = tempfile.mkdtemp()
|
||||
self.secrets_file = Path(self.temp_dir) / ".secrets.toml"
|
||||
|
||||
def teardown_method(self):
|
||||
"""Clean up temporary files after each test."""
|
||||
import shutil
|
||||
|
||||
if hasattr(self, "temp_dir") and Path(self.temp_dir).exists():
|
||||
shutil.rmtree(self.temp_dir)
|
||||
|
||||
def create_secrets_toml(self, personal_access_token="initial_token"):
|
||||
"""Create a .secrets.toml file with GitLab credentials."""
|
||||
content = f"""[gitlab]
|
||||
personal_access_token = "{personal_access_token}"
|
||||
"""
|
||||
self.secrets_file.write_text(content)
|
||||
|
||||
def test_gitlab_credentials_not_cached_when_fresh(self):
|
||||
"""
|
||||
Test that GitLab credentials are not cached when marked as fresh.
|
||||
|
||||
This verifies the core requirement: when GITLAB is in fresh_vars,
|
||||
accessing the credentials multiple times should reload from disk
|
||||
each time, not return a cached value.
|
||||
"""
|
||||
# Create initial secrets file
|
||||
self.create_secrets_toml(personal_access_token="no_cache_v1")
|
||||
|
||||
# Set FRESH_VARS_FOR_DYNACONF environment variable
|
||||
with patch.dict(os.environ, {"FRESH_VARS_FOR_DYNACONF": '["GITLAB"]'}):
|
||||
# Create Dynaconf with GITLAB marked as fresh via env var
|
||||
settings = create_dynaconf_with_custom_loader(self.temp_dir, self.secrets_file)
|
||||
|
||||
# Access the token multiple times before modification
|
||||
access_1 = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
access_2 = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
access_3 = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
|
||||
# All should return the same value (file hasn't changed)
|
||||
assert access_1 == access_2 == access_3 == "no_cache_v1", (
|
||||
"Multiple accesses before modification should return same value"
|
||||
)
|
||||
|
||||
# Modify the file
|
||||
self.create_secrets_toml(personal_access_token="no_cache_v2")
|
||||
|
||||
# Access again - should get new value immediately
|
||||
access_4 = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
assert access_4 == "no_cache_v2", "First access after modification should return new value"
|
||||
|
||||
# Verify no caching occurred
|
||||
assert access_1 != access_4, "Value should change after file modification (no caching)"
|
||||
|
||||
# Modify again
|
||||
self.create_secrets_toml(personal_access_token="no_cache_v3")
|
||||
|
||||
# Access again - should get newest value
|
||||
access_5 = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
assert access_5 == "no_cache_v3", "Second modification should also be detected"
|
||||
|
||||
# Verify the progression
|
||||
assert access_1 != access_4 != access_5, "Each modification should result in a different value (no caching)"
|
||||
|
||||
def test_fresh_vars_works_with_default_loaders(self):
|
||||
"""
|
||||
Test that fresh_vars works correctly with Dynaconf's default core loaders.
|
||||
|
||||
This is a control test to prove that fresh_vars functionality works
|
||||
as expected when using the standard Dynaconf configuration (with core_loaders).
|
||||
This helps isolate the bug to the custom_merge_loader configuration.
|
||||
"""
|
||||
# Create initial secrets file
|
||||
self.create_secrets_toml(personal_access_token="default_v1")
|
||||
|
||||
# Create Dynaconf with DEFAULT loaders (not custom_merge_loader)
|
||||
settings = Dynaconf(
|
||||
# Use default core_loaders (don't disable them)
|
||||
root_path=self.temp_dir,
|
||||
merge_enabled=True,
|
||||
envvar_prefix=False,
|
||||
load_dotenv=False,
|
||||
settings_files=[str(self.secrets_file)],
|
||||
fresh_vars=["GITLAB"],
|
||||
)
|
||||
|
||||
# First access
|
||||
first_value = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
assert first_value == "default_v1"
|
||||
|
||||
# Modify file
|
||||
self.create_secrets_toml(personal_access_token="default_v2")
|
||||
|
||||
# Second access - should be reloaded with default loaders
|
||||
second_value = settings.GITLAB.PERSONAL_ACCESS_TOKEN
|
||||
assert second_value == "default_v2", (
|
||||
"With default loaders, fresh_vars SHOULD work correctly. "
|
||||
"If this test fails, the issue is not specific to custom_merge_loader."
|
||||
)
|
||||
|
||||
assert first_value != second_value, "Values should be different when using default loaders with fresh_vars"
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
pytest.main([__file__, "-v"])
|
||||
Loading…
Reference in a new issue