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

internal/ini: Fix ini parser to handle empty values #2860

Merged
merged 12 commits into from
Oct 1, 2019

Conversation

skotambkar
Copy link
Contributor

@skotambkar skotambkar commented Sep 27, 2019

The ini parser incorrectly decided whether a statement should be skipped. As a result, valid statements in the ini files were being squashed. The PR fixes incorrect modifications to the previous token value of the skipper. We also add checks for cases where a skipped statement should be marked as complete and not be ignored.

Adds test cases for cases for statements that need to be skipped. Also adds suggested tests from #2801 . Fixes #2800

@jasdel
Copy link
Contributor

jasdel commented Sep 27, 2019

I think the failing Go 1.7 test is fixed in PR #2841 once that pr is merged should be able to rebase and rerun tests

Copy link
Contributor

@jasdel jasdel left a comment

Choose a reason for hiding this comment

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

Could you update the description on why the ini parser change fixes the issue? This might help investigate future issues.

also should this PR include the ini_parser_test.go update in #2801?

internal/ini/testdata/valid/issue_2800_expected Outdated Show resolved Hide resolved
@skotambkar skotambkar force-pushed the iniParser branch 3 times, most recently from b8f048b to 20c815f Compare September 28, 2019 20:22
jasdel and others added 4 commits September 28, 2019 14:04
Ensure HTTP servers are cleaned up during testing after no longer needed. This was creating excessive noise in testing stack traces due to the number of background goroutines active.

Also updated SDK's unit tests to use verbose logging, have shorter runtime, and have a timeout fo 5m.

Related to aws#2840
@skotambkar skotambkar self-assigned this Sep 30, 2019
@skotambkar skotambkar merged commit 02207b1 into aws:master Oct 1, 2019
@skotambkar skotambkar deleted the iniParser branch October 1, 2019 21:32
@aws-sdk-go-automation aws-sdk-go-automation mentioned this pull request Oct 2, 2019
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.

internal/ini: INI Parser Fails Subsequent Section Parsing With Missing Right Hand Value in Previous Section
3 participants