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

Reactivate some of the unit tests #146

Closed
wants to merge 9 commits into from
Closed

Reactivate some of the unit tests #146

wants to merge 9 commits into from

Conversation

jayaddison
Copy link
Contributor

Description

Reactivates some of Gourmand's unit tests, largely by picking up some work-in-progress by @MrShark.

How Has This Been Tested?

Run locally using pytest; some test modules were run individually to check whether they pass in isolation from the rest of the test suite (not all do: some of the Importer modules in particular seem to rely on database setup that occurs when other Python modules are loaded during test suite run)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

MrShark and others added 7 commits August 14, 2022 12:53
Handle more types of input (that is used in the code)
* If no filename, don't backup
* if the file is a string, convert it to a Path

Make the backup non interactive, since the functionality is not exposed
in the gui and used in tests.
Reactivate test_db, test_exportManager, test_importer and test_web_importer
since fixes to backup_database makes most tests run again in those
files.

In test_db remove some magic constants for initial database to make
tests run.
* Use pytest instead of unittest
* PEP8 formating
* Variable naming to make easier to read
* Use "proper" unicode characters instead of binary strings
…ot appear to pass locally without a database instance available
…ected behaviour for test_convert.testAdjustments?)
…nteractive_importer module fails when run individually, but passes when run as part of a larger pytest suite?)

while backup_name.is_file():
backup_name = backup_name.with_name(backup_name.name + 'I')

show_message(
Copy link
Contributor Author

@jayaddison jayaddison Mar 10, 2023

Choose a reason for hiding this comment

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

Todo:

  • As noted, this informational message needs to be restored elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@jayaddison jayaddison Mar 28, 2023

Choose a reason for hiding this comment

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

What do you think about the approach taken in feb0d5a @cydanil @MrShark?

It uses a SQLite 'in-memory' URI for the database. That means that self.filename for the database is empty, and the backup is skipped.

It feels a little fragile - it's relying on a series of interconnected things fitting together. But it works (tests pass, and the UI doesn't display the 'backup' message -- but should still do during interactive usage when a database upgrade occurs).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(I'd probably like to attempt to find a better solution, but until then.. review & feedback welcome)

…(it'll likely change, but this is me trying to understand what the problem with it is during testing)

Partially reverts commit b6b8d18.
@jayaddison jayaddison marked this pull request as ready for review March 28, 2023 22:32
@jayaddison
Copy link
Contributor Author

@cydanil a notification to let you know that I'm closing this pull request and deleting the branch / source -- not because I sense any problem with it as-is, but I have this background tendency to want to remove old/stale projects and code that I'm not actively developing and/or that don't seem to be going anywhere (it's a bit like removing unnecessary global variables, or something - reducing the working memory of things to context-switch between). You're welcome to include and use/merge the code if you like, but no problem if not! (lengthy explanation for probably a trivial action, I know :))

@jayaddison jayaddison closed this Jun 19, 2023
@jayaddison
Copy link
Contributor Author

I have this background tendency to want to remove old/stale projects and code that I'm not actively developing and/or that don't seem to be going anywhere

And by this I purely refer to this pull request! I should've been clearer. I hope all the best for Gourmand :)

@jayaddison jayaddison deleted the issue-137/reactivating-some-unit-tests branch June 19, 2023 12:39
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.

2 participants