Buried-In-Code / Dex-Starr

Dex-Starr helps sort and organize your comic collection by using the information stored in ComicInfo files
GNU General Public License v3.0
0 stars 2 forks source link

Unable to load Config Files during Github Action testing #16

Closed Buried-In-Code closed 3 years ago

Buried-In-Code commented 3 years ago

Currently to test the script a config.ini file is required. Need to investigate how to load one for Github Actions to be able to load

bpepple commented 3 years ago

Couple of ways to solve this, one is to make your configparser not use a hard-coded file location. You then could create a test-config to use when running test. The second option would Mock any values needed to run the tests. Without seeing the GH failure logs, and I'm not sure which route is better solution.

Buried-In-Code commented 3 years ago

After taking a closer look at how I implemented the tests rather than loading a test config, they try and load it how Comic-Organizer would for any user. After thinking about it, i dont need to have it implemented that way and should instead pass the values to the tests via Environment variables, like you do for Mokkari tests.

bpepple commented 3 years ago

After taking a closer look at how I implemented the tests rather than loading a test config, they try and load it how Comic-Organizer would for any user.

Really, I'd say this is actually a bug that was exposed.

By hard-coding a config file location without any type of error handling you're making this section of the code fairly brittle. A better solution would be to gracefully exit if the file isn't found, or to create an empty file and then error out if any info is missing when needed. Really all those variables (COMICVINE_API_KEY, LEAGUE_API_KEY, etc) should be using default values (None, "", etc) if for some reason their keys are not in config file.

There are a lot of different ways to implement this depending on your needs, for most of my simple projects I tend to use something like this:

https://github.com/bpepple/metron-tagger/blob/master/metrontagger/taggerlib/settings.py

Anyway, something to think about.