Skip to content

Limit tox to < 4.9#8443

Merged
auvipy merged 1 commit intocelery:mainfrom
Katz-Consulting-Group:hotfix
Aug 19, 2023
Merged

Limit tox to < 4.9#8443
auvipy merged 1 commit intocelery:mainfrom
Katz-Consulting-Group:hotfix

Conversation

@Nusnus
Copy link
Copy Markdown
Member

@Nusnus Nusnus commented Aug 17, 2023 •
edited
Loading

Tox has released 4.9.0 which fixed a bug where non existing environments did NOT fail, so since tox 4.9.0 they now DO fail: tox-dev/tox#2858

We are relying on the previous behavior (the bugged one) so until we can fix it, I'm limiting tox as it breaks every PR right now which is worse than the limitation itself (which is not recommended).

We need to fix this soon so we can remove this limitation @auvipy @thedrow

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 17, 2023 •
edited
Loading

Codecov Report

Patch and project coverage have no change.

Comparison is base (5a724ac) 87.42% compared to head (cb8495a) 87.42%.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #8443   +/-   ##
=======================================
  Coverage   87.42%   87.42%           
=======================================
  Files         148      148           
  Lines       18479    18479           
  Branches     3154     3154           
=======================================
  Hits        16155    16155           
  Misses       2034     2034           
  Partials      290      290
Flag Coverage Δ
unittests 87.39% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@Nusnus Nusnus changed the title tmp Added allowlist_externals = * to tox.ini due to tox 4.9.0+ Aug 17, 2023
@Nusnus Nusnus changed the title Added allowlist_externals = * to tox.ini due to tox 4.9.0+ Added allowlist_externals = * Aug 17, 2023
@Nusnus Nusnus force-pushed the hotfix branch 2 times, most recently from e86921f to 80e041f Compare August 17, 2023 17:29
@Nusnus Nusnus changed the title Added allowlist_externals = * Added allowlist_externals = * to tox.ini Aug 17, 2023
@Nusnus Nusnus changed the title Added allowlist_externals = * to tox.ini Trying to find out what’s wrong with tox in GitHub Aug 17, 2023
@Nusnus Nusnus force-pushed the hotfix branch 3 times, most recently from fe4b1dd to 8461491 Compare August 17, 2023 20:36
@auvipy
Copy link
Copy Markdown
Member

auvipy commented Aug 18, 2023 •
edited
Loading

might be useful https://tox.wiki/en/latest/changelog.html and should we consider finding out alternative like https://github.com/tox-dev/tox-gh & https://github.com/tox-dev/tox-docker?

@Nusnus Nusnus force-pushed the hotfix branch 3 times, most recently from 196cf59 to 0dd6103 Compare August 18, 2023 14:43
@auvipy auvipy self-requested a review August 18, 2023 14:47
auvipy
auvipy reviewed Aug 18, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

seems working

@Nusnus Nusnus changed the title Trying to find out what’s wrong with tox in GitHub Limit tox to < 4.9 Aug 18, 2023
@Nusnus Nusnus requested a review from auvipy August 18, 2023 15:24
@Nusnus Nusnus self-assigned this Aug 18, 2023
@Nusnus Nusnus added this to the 5.3.x milestone Aug 18, 2023
@Nusnus
Copy link
Copy Markdown
Member Author

Nusnus commented Aug 18, 2023

I've been investigating this issue & redis 5.x issue in the last few days, what timing hehe..
Anyways, @auvipy & @thedrow, FYI, we're now limited to tox <4.9 and Redis < 5.

These are not major issues at the moment but 1) be aware, 2) we need to remove these limitations soon as the time will come when upgrading will be mandatory so let's keep that in mind.

@Nusnus Nusnus marked this pull request as ready for review August 18, 2023 15:34
@Nusnus
Copy link
Copy Markdown
Member Author

Nusnus commented Aug 18, 2023

We are relying on the previous behavior (the bugged one) ...

Meaning we lack integration tests support in tox.ini under [gh-actions] section

@Nusnus Nusnus marked this pull request as draft August 19, 2023 12:55
@Nusnus
Tox v4.9 has fixed a bug where unsupported environments
cb8495a 
did not raise an error but now they do. As our tox.ini
only implement unit tests config for tox-gh-actions, since
tox v4.9 our integration tests fail on GitHub. This change limits
tox to v4.9 until we can fix it correctly as it breaks the
testing environment for now
@auvipy auvipy marked this pull request as ready for review August 19, 2023 13:33
auvipy
auvipy approved these changes Aug 19, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

I'm going to merge this for now. to keep the CI working. we have to rethink the way we configured integration tests in tox.

@auvipy auvipy merged commit 372a7a3 into celery:main Aug 19, 2023
@auvipy auvipy added the CI label Aug 19, 2023
@Nusnus
Copy link
Copy Markdown
Member Author

Nusnus commented Aug 19, 2023

@auvipy

I'm going to merge this for now. to keep the CI working. we have to rethink the way we configured integration tests in tox.

I've improved my fix and we also have 100% passing tests so this is good to go indeed.

Regarding the future, I guess I'll be able to get to it once I go back to developing the smoke tests PR as it will suffer the same issues as the integration tests, or maybe earlier, we'll see.

@Nusnus Nusnus deleted the hotfix branch August 19, 2023 13:53
Nusnus added a commit to Katz-Consulting-Group/celery that referenced this pull request Aug 30, 2023
@Nusnus
Limit tox to <4.9 in pkgutils.txt - missed that in celery#8443
5451d48
@Nusnus Nusnus mentioned this pull request Aug 30, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@auvipy auvipy auvipy approved these changes

Assignees

@Nusnus Nusnus

Labels

CI

Projects

None yet

Milestone

5.3.x

Development

Successfully merging this pull request may close these issues.

2 participants

@Nusnus @auvipy