replacing flags package to read from env variables, updating README

[deranjer]

This allows reading in env variables to set goStatic arguments.

Updated README with some more info.

Was not able to test this in Docker (not too familiar with the build process) but this does work locally on Windows when I tested it.

[deranjer]

Well just realized I may not have organized the additional Docker information in the README in the most optimal way, but can change that if requested.

[PierreZ]

Thanks for the PR 🚀

Left some comments, but I would love to have a test to check if flag's content is loaded through env var. Do you see how we could achieve this?

[PierreZ]

Maybe "Use goStatic with Docker" is a better name? Also, H3 title may be better in the Usage part

[PierreZ]

Maybe "Use goStatic with Docker-compose" is a better name?

[PierreZ]

Could you add an example?

[deranjer]

I don't see an easy way, but am not terribly experienced with testing in golang. Just about everything is in the main function. One possible option is to split up the main function up into multiple parts and try testing it that way. Still not the easiest, but I think it could be done.

The other option is somehow calling the main func from testing (not sure if that is even possible considering it stands up a server) or building a binary and running tests against that while handling killing the process after we test. I took an initial crack at that, but was not successful.

If we abandon golang testing entirely we would need to find an external tool to run the executable through its paces (most likely in a build pipeline) and check the output vs the expected output.

Let me know your thoughts on this. I'm sure there is something I'm missing since I'm not terribly familiar with testing in golang.

[PierreZ]

I don't see an easy way, but am not terribly experienced with testing in golang. Just about everything is in the main function. One possible option is to split up the main function up into multiple parts and try testing it that way. Still not the easiest, but I think it could be done.

The other option is somehow calling the main func from testing (not sure if that is even possible considering it stands up a server) or building a binary and running tests against that while handling killing the process after we test. I took an initial crack at that, but was not successful.

If we abandon golang testing entirely we would need to find an external tool to run the executable through its paces (most likely in a build pipeline) and check the output vs the expected output.

Let me know your thoughts on this. I'm sure there is something I'm missing since I'm not terribly familiar with testing in golang.

Don't worry, as you are the first one to add test on goStatic, I'm expecting something simple for this PR 😉

Something like this in the main_test.go should do the trick:

package main

import "testing"

func TestFlags(t *testing.T) {
  // TODO: set multiples envs with https://pkg.go.dev/os#Setenv (some of the flags, maybe not all)
  flag.Parse()
  // check that variables are in good shape
}

This will allow me(us?) to have some simple regression tests 🚀

Let me know if you have some questions, my go is rusty, but I would be happy to help you with that.

[deranjer]

Well it was good to do some more full testing. Unfortunately, one of the params is "path" which fails testing (assuming because PATH is already set in pretty much every OS). Maybe with docker it might be different, but it certainly fails the tests. There is an option to have a prefix in flags, but was unable to get that working. I'll play with it a little more, but might end up canceling this pull since it would require renaming the "path" parameter.

[PierreZ]

Well it was good to do some more full testing. Unfortunately, one of the params is "path" which fails testing (assuming because PATH is already set in pretty much every OS). Maybe with docker it might be different, but it certainly fails the tests. There is an option to have a prefix in flags, but was unable to get that working. I'll play with it a little more, but might end up canceling this pull since it would require renaming the "path" parameter.

I can help you if needed

[harveysanders]

Resolves #50

Just throwing this on here to link the PR to the issue. I was going to take a look before I noticed this PR : )