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

Fix reminders unit tests; refs #1614 #376

Closed

Conversation

gagnieray
Copy link
Collaborator

No description provided.

@trasher
Copy link
Member

trasher commented Dec 4, 2023

Thank you very much, I'll take a look

@codecov-commenter
Copy link

codecov-commenter commented Dec 5, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (402e9e7) 44.44% compared to head (887f030) 44.44%.

❗ Current head 887f030 differs from pull request most recent head f290ff2. Consider uploading reports for the commit f290ff2 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@                  Coverage Diff                   @@
##             feature/fix/1614     #376      +/-   ##
======================================================
- Coverage               44.44%   44.44%   -0.01%     
  Complexity               5880     5880              
======================================================
  Files                     142      142              
  Lines                   23886    23885       -1     
======================================================
- Hits                    10616    10615       -1     
  Misses                  13270    13270              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@trasher
Copy link
Member

trasher commented Dec 6, 2023

My latest commit fixes tests; could you confirm it seems working as expected on your side @gagnieray?

@gagnieray
Copy link
Collaborator Author

My latest commit fixes tests; could you confirm it seems working as expected on your side @gagnieray?

Unfortunately, no 😕

I wrote more tests. But they do not fail as expected (or at least not in the same conditions as things are failing on my instance). So I wonder if I wrote them correctly 🤔

Please, have a look at my latest commit.
And compare the results of the latest tests on github with my own below :

  1. On case n°1 :

    // #1 : test reminders
    //
    // Both members should be up-to-date and receive ALL the reminders.

    • first impending reminders are properly sent
    • second impending reminders fail
    • first late reminders fail
    • second late reminders are properly sent
  2. On case n°2 :

    // #2 : test reminders
    //
    // Only Member 1 renewed membership last year after last late reminder.
    // Only Member 1 should now be up-to-date and receive ALL the reminders.
    //
    // For Member 2, if its user account has not been set to inactive, he/she
    // should be outdated and receive only one late reminder this year.

    • first impending reminders are properly sent
    • second impending reminders fail
    • first late reminders fail : only one out of two reminders is sent for the member late for more than 1 year
    • second late reminders are properly sent
  3. On case n°3 :

    // #3 : test reminders
    //
    // Member 2 has already received ALL reminders this year and
    // is now outdated and should not receive other reminders until
    // next membership period.

    • first impending reminder is properly sent
    • second impending reminder fails
    • first late reminder fails
    • second late reminder is properly sent

@trasher
Copy link
Member

trasher commented Dec 7, 2023

My latest commit fixes tests; could you confirm it seems working as expected on your side @gagnieray?

Unfortunately, no 😕

I wrote more tests. But they do not fail as expected (or at least not in the same conditions as things are failing on my instance). So I wonder if I wrote them correctly 🤔
[...]

After a quick review, I did no see anything wrong in your tests; but I probably need to spend much more time on that.

I've released 1.0.0 final, reminder issue is very old; I'll release a bugfixes version once it will be fixed.

I'll take a closer look later, thanks for the tests updates; maybe will I finally understand what's wrong :/

@gagnieray
Copy link
Collaborator Author

maybe will I finally understand what's wrong :/

I hope so ! 😄

I found new "clues". Please have a look at https://bugs.galette.eu/issues/1614#note-13

@trasher
Copy link
Member

trasher commented Dec 17, 2023

I've tried to understand why your latest commit make tests fails; but the only conclusion I got is tests expectations are currently wrong :D
But I also must admit I'm a bit lost in this test case... I'll probably revert your latest change to and work on a brand new test case reproducing what you describe on the tracker ticket.

Xmas approaching, I'm not sure I can take an eye before next year :)

@trasher
Copy link
Member

trasher commented Dec 22, 2023

1st and 2nd commits have been merged in #373; third commit has been ignored as I said, and last one is already in develop branch :)

@trasher trasher closed this Dec 22, 2023
@gagnieray gagnieray deleted the feature/fix/1614-tests branch December 22, 2023 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants