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 errors in GH action coverage test #323

Merged
merged 4 commits into from
Dec 8, 2023
Merged

Conversation

Xpirix
Copy link
Collaborator

@Xpirix Xpirix commented Dec 1, 2023

Summary of this PR:

  • Update some packages (django-taggit, 'Whoosh', 'picke5') in requirements to fix errors when running tests or commands in a development environment
  • Update docker-compose to fix the make error in a development environment
  • Fix test_plugin_list and test_plugin_version_feedback to reflect the code. In test_plugin_version_feedback, <div class="span8 new-feedback"> becomes <div class="new-feedback"> because <div class="span8 new-feedback"> doesn't exist and the test will always fail.

@Akiyamka
Copy link
Contributor

Akiyamka commented Dec 5, 2023

I ran this code after the fixes and it works fine now. Tested scripts:

$ make build
$ make web
$ make devweb
$ make migrate
$ make devweb-runserver
$ pip install -r REQUIREMENTS-dev.txt
$ pre-commit install --config .pre-commit-config.yaml

http://0.0.0.0:62202/admin/login/?next=/admin/ shows login screen.

However, when I open root http://0.0.0.0:62202/, I see error page

DoesNotExist at /
Menu matching query does not exist.
Error during template rendering
{% get_namedmenu Navigation as menu %}

is this expected behaviour?

@Xpirix
Copy link
Collaborator Author

Xpirix commented Dec 6, 2023

I think it's normal when DB is empty. Running make dbseed will probably fix this.

REQUIREMENTS.txt Outdated
Whoosh==2.5.6

# Updates for Django 2 & Python 3.7
git+https://github.com/Xpirix/whoosh.git
Copy link

Choose a reason for hiding this comment

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

Wouldn't it be better to specify a specific commit?

@@ -12,7 +12,7 @@ RUN echo "deb http://archive.debian.org/debian stretch main contrib non-free" >
RUN apt-get update && apt-get install -y libsasl2-dev python-dev libldap2-dev libssl-dev
ADD REQUIREMENTS.txt /REQUIREMENTS.txt
RUN pip install -r /REQUIREMENTS.txt
RUN pip install uwsgi
RUN pip install uwsgi freezegun
Copy link

Choose a reason for hiding this comment

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

Wouldn't it be better to specify a specific version?

django-haystack

# Updates for Django 2 & Python 3.7
git+https://github.com/Xpirix/whoosh.git
Copy link

@dqunbp dqunbp Dec 6, 2023

Choose a reason for hiding this comment

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

Same here

django-haystack

# Updates for Django 2 & Python 3.7
git+https://github.com/Xpirix/whoosh.git
Copy link

Choose a reason for hiding this comment

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

Same here

@Xpirix
Copy link
Collaborator Author

Xpirix commented Dec 6, 2023

@dqunbp . Thanks for the feedback. Yes, I also think it's better to specify the commit or the version to prevent errors in the future. I've just made these fixes.

@dimasciput dimasciput merged commit 6589e3c into qgis:master Dec 8, 2023
2 checks passed
@Xpirix Xpirix deleted the fix_gh_test branch January 2, 2024 06:21
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.

None yet

4 participants