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

Add dotenv tests #778

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ParetoOptimalDev
Copy link

It's failing as expected when run with nix eval .#tests.

This will also soon fix #772

It's failing as expected when run with `nix eval .#tests`
@ParetoOptimalDev
Copy link
Author

ParetoOptimalDev commented Aug 7, 2023

@domenkozar This is very prelimnary, but I just wanted to get something up and working.

I'm guessing you want to structure things like dotenv-functions differently, perhaps even having the test somehow use modules/integrations/dotenv.nix rather than introduction modules/integrations/dotenv-functions.nix.

Or perhaps it's a positive to have a module not requiring imports that is more pure?

Also obviously need to wire this into github actions, not sure if you had plans beyond just calling nix eval .#tests for that.

I also don't know the regex you'd prefer using, but I can attempt to figure out a better one than the one I currently have (which might mean just seeing what python-dotenv does). If you already have one in mind though, I can just use that.

@domenkozar
Copy link
Member

Looks good, we can refactor once the tests pass 🔥

@ParetoOptimalDev
Copy link
Author

ParetoOptimalDev commented Aug 7, 2023

I'm having problems getting the regex to optionally match surrounding single quotes.

I'm pretty close though with:

https://regex101.com/r/ch9iSw/1

^'?(.*?)'? *= *'?(.*?)'?

I think the issue is with the optional space matching, followed by optional quote, followed by lazy matching any character. The pattern works on the first one because it knows the beginning of the line I believe.

Here is the current one for reference that doesn't handle quotes in regex101: https://regex101.com/r/qgtFkK/1

@ParetoOptimalDev
Copy link
Author

I think the issue is with the optional space matching, followed by optional quote, followed by lazy matching any character. The pattern works on the first one because it knows the beginning of the line I believe.

queue thought "what if we mark the end of the line!"

Yep, that works: https://regex101.com/r/Tc8L3x/1

@ParetoOptimalDev
Copy link
Author

Nevermind, nix and this regex test differ in result:

https://regex101.com/r/DOlw5S/1

nix-repl> builtins.match "'?(.*?)'? *= *'?([^'\n]*)" "a='{\"a\":\"b\"}'"
builtins.match "'?(.*?)'? *= *'?([^'\n]*)" "a='{\"a\":\"b\"}'"
null

@domenkozar
Copy link
Member

What's missing here?

@ParetoOptimalDev
Copy link
Author

  • getting the regex to work for the simple cases so far
  • finish tests, likely needing to tune regex along the way
  • this to become higher priority for me over other tasks

This was high priority for me while I thought I could transition to it at work from docker-compose.

However I got stuck on #782, created a reproduction, and ran out of time and had to revert to using docker-compose again.

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.

dotenv integration captures quotes in value/improperly stores json
2 participants