-
Notifications
You must be signed in to change notification settings - Fork 39
add non-ascii connection string tests #63
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 non-ascii connection string tests #63
Conversation
Actually, the error is reproducible if you change the dependencies:
buffer: ^1.1.0
crypto: ^3.0.0
collection: ^1.15.0
sasl_scram: '0.1.0' # <~~~~ pin here In this version, both
In my local When #38 was filed (and when I experienced issues with See: |
void main() { | ||
/// This cannot be run on CI as each test requires different pg_hba.conf | ||
/// to be mounted to the container. | ||
if (Platform.environment.containsKey('GITHUB_ACTION')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't really checked yet, but IIRC Github Action is able to run docker run
and other commands... have you tried it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I believe it's possible but not with the current setup (e.g. using services.postgres
)... I'll give it another shot later and try a different setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: a repository can have multiple workflows, and we can differentiate tests either via directory or tags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Also: thank you for looking into this!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that what I was thinking -- to tag tests with a solo
tag and exclude them from common tests and then run all solo
tests with concurrency=1
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the continued work here! I'll just merge this and we can figure out how to run it on Github actions later.
I was taking a look at #38 but I could not reproduce the issue (actually I remember facing issues with
@
symbol a while back as well but I could not reproduce that either).Anyway, I've already added tests for passwords using non-ASCII characters for the three supported auth methods by the package -- namely:
All of them pass (won't work on the CI though as each test needs its own
pg_hba.conf
to be mounted).They might be useful to keep if you think so.-- I believe they are necessary based on the next comment.