Remove reference to PY2 and add testing to github actions

pallets-eco / flask-session

Server side session extension for Flask

https://flask-session.readthedocs.io

BSD 3-Clause "New" or "Revised" License

490 stars 236 forks source link

Remove reference to PY2 and add testing to github actions #179

Closed darless closed 5 months ago

darless commented 1 year ago

This closes #166

Removes references to PY2, since we only use python3
Removed the current test file and instead created tests in a new tests directory
Added test github actions workflow to use pytest to run the new tests
Created pytest.txt in requirements directory which is used for the github actions CI

darless commented 10 months ago

@ThiefMaster what else is needed for PR to be merged?

ThiefMaster commented 10 months ago

I'm not a maintainer of flask-session

so i'll leave that up to @pallets-eco/flask-session

PS: check the red icon below some files in the PR diff view. that indicates those files do not have a trailing linebreak. this should probably be fixed...

darless commented 10 months ago

@ThiefMaster I'm more than happy to ping the maintainer. When I look at contributors I only see those that commit to repo, but not the owner or those that have write access. Can you let me know how I can see that, and I'll ping them? I fixed the minor newline.

ThiefMaster commented 10 months ago

I pinged them in my last comment

Lxstr commented 6 months ago

Hi @darless and @ThiefMaster

Thanks for this great PR. It has made things much easier after the difficulty in merging #170. I tried merging this and #188 in a development branch based on the last release (rather than from that merge) to see if that was a better path. I think it has gone well, pending some testing. I believe the recurring issues related to writing empty sessions and unneeded overwrites may be resolved.

We would then somehow reset/revert main and then continue to cherry pick other features and fixes from flask-session2 as required, and as we understand them more fully.

In this attempt, I have opted for your Pytest implementation over the refactor done in #188. However, I am not very familiar with it and having issues with the error assertion as you can see in https://github.com/pallets-eco/flask-session/actions/runs/7419702330, would you be able to assist?

Lxstr commented 6 months ago

I was also thinking it would be great to have tests that cover "nothing" endpoints first, where session isn't set or modified, to confirm that no session is unnecessarily set as per #193 and others. What are your thoughts?