Skip to content

[16.0][FIX] hr_holidays_leave_auto_approve: fix test w/ new validation - #231

Open
pcastelovigo wants to merge 1 commit into
OCA:16.0from
flinq-ingenieria:16.0-fix-test-hr_holidays_leave_auto_approve
Open

[16.0][FIX] hr_holidays_leave_auto_approve: fix test w/ new validation#231
pcastelovigo wants to merge 1 commit into
OCA:16.0from
flinq-ingenieria:16.0-fix-test-hr_holidays_leave_auto_approve

Conversation

@pcastelovigo

@pcastelovigo pcastelovigo commented Nov 29, 2025

Copy link
Copy Markdown

Unit test tested function asking for a leave using datetime.today.
If CI was run in saturday or sunday, asking for a leave raised a ValidationError as it is not a working day.

I modified the test to be sure that selected day is between monday and friday, making test to be valid also when run on weekends

Fixes #230 intermittent error

@pcastelovigo
pcastelovigo force-pushed the 16.0-fix-test-hr_holidays_leave_auto_approve branch from 48bce57 to b0e4b81 Compare December 9, 2025 15:02

@marcos-mendez marcos-mendez left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated Review -- Tests Failed

1. Root Cause

The test failure is caused by a database connection error during test execution, likely due to an invalid or missing database configuration in the test environment (e.g., runboat infrastructure). This is not caused by the code changes in the PR but rather by the test environment setup.

2. Suggested Fix

There is no code fix needed for the PR itself, as the issue is not in the logic of the tests or the module code. However, ensure that the test environment is correctly configured to connect to a valid PostgreSQL database. If the issue persists, check:

  • Database URL in odoo.conf or environment variables.
  • Network access to the database from the test runner.
  • Docker or Kubernetes configuration for the runboat environment.

3. Additional Code Issues

No real bugs found in the provided diff. The changes in test_hr_holidays_leave_auto_approve.py are valid:

  • The use of while today.weekday() >= 5: ensures that test dates are not weekends, which is a reasonable test practice.
  • No OCA patterns violated; this is a standard test date handling improvement.

4. Test Improvements

To improve test coverage and robustness, consider the following:

Add tests for edge cases:

  • Test leave requests created on weekends (ensure they are handled gracefully).
  • Test auto-approval behavior for different leave types and employee roles.
  • Test allocation requests with zero duration or invalid dates.

Suggested test patterns:

Use TransactionCase or SavepointCase for tests involving:

  • with_user() context changes.
  • State transitions and auto-approval logic.
  • Multi-user scenarios.

Example:

def test_leave_request_auto_approve_on_monday(self):
    # Ensure the test date is a Monday
    monday = datetime.today() - timedelta(days=datetime.today().weekday())
    # Create leave request
    leave = self.leave_request_model.create({
        'name': 'Test Leave',
        'date_from': monday,
        'date_to': monday,
        'holiday_type': 'employee',
        'employee_id': self.employee.id,
    })
    self.assertEqual(leave.state, 'approved')  # Assuming auto-approval is enabled

This improves test reliability and ensures that auto-approval works as expected under various conditions.


⏰ PR Aging Alert

This PR by @pcastelovigo has been open for 105 days (3 months).
🔴 Zero human reviews in 105 days. This contributor invested their time to improve this module. The PSC owes them at least a response — even a "needs changes" is better than silence.
💤 No activity for 96 days. Has this PR been forgotten?

Every ignored PR is a contributor who might not come back. Review time matters. (OCA Aging Report)


Reciprocal Review Request

Hi everyone! I found some test failures on this PR and left detailed feedback above. I am happy to discuss or help debug. In the meantime, if any of you get a chance, I would appreciate a look at my open PR(s):

My open PRs across OCA:

Reviewing each other's work helps the whole community move forward. Thank you!


Environment via OCA Neural Reviewer: Minikube + K8s Job + oca-ci/py3.10-odoo16.0 | Odoo 16.0
Automated review by OCA Neural Reviewer + qwen3-coder:30b

@NL66278 NL66278 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍 LGTM Hope somebody can merge this fast, as it is also causing problems in other PR's.

@NL66278

NL66278 commented May 8, 2026

Copy link
Copy Markdown

@pcastelovigo Please discard the "review" by marcos-mendez as it is just AI slop.

@pcastelovigo

Copy link
Copy Markdown
Author

@pcastelovigo Please discard the "review" by marcos-mendez as it is just AI slop.

i have no permissions

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.

3 participants