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

test1148: tolerate progress updates better (again) #5194

Closed
wants to merge 1 commit into from

Conversation

@jay
Copy link
Member

jay commented Apr 7, 2020

  • Ignore intermediate progress updates.

  • Support locales that use a character other than period as decimal
    separator (eg 100,0%).

test1148 checks that the progress finishes at 100% and has the right
bar width. Prior to this change the test assumed that the only progress
reported for such a quick transfer was 100%, however in rare instances
(like in the CI where transfer time can slow considerably) there may be
intermediate updates. For example, below is stderrlog1148 from a failed
CI run with explicit \r and \n added (it is one line; broken up so that
it's easier to understand).

\r
\r##################################                                        48.3%
\r######################################################################## 100.0%
\n

Closes #xxxx


The improvements made in 6cbe969 (#2488) didn't go far enough.

CI failure: https://github.com/jay/curl/runs/565278274#step:6:2297

To reproduce:

diff --git a/src/tool_cb_prg.c b/src/tool_cb_prg.c
index aad451b..77d892e 100644
--- a/src/tool_cb_prg.c
+++ b/src/tool_cb_prg.c
@@ -194,6 +194,8 @@ int tool_progress_cb(void *clientp,
       num = MAX_BARLENGTH;
     memset(line, '#', num);
     line[num] = '\0';
+    /* !checksrc! disable LONGLINE 1 */
+    fprintf(bar->out, "%s", "\r##################################                                        48.3%");
     msnprintf(format, sizeof(format), "\r%%-%ds %%5.1f%%%%", barwidth);
     fprintf(bar->out, format, line, percent);
   }
- Ignore intermediate progress updates.

- Support locales that use a character other than period as decimal
  separator (eg 100,0%).

test1148 checks that the progress finishes at 100% and has the right
bar width. Prior to this change the test assumed that the only progress
reported for such a quick transfer was 100%, however in rare instances
(like in the CI where transfer time can slow considerably) there may be
intermediate updates. For example, below is stderrlog1148 from a failed
CI run with explicit \r and \n added (it is one line; broken up so that
it's easier to understand).

\r
\r##################################                                        48.3%
\r######################################################################## 100.0%
\n

Closes #xxxx
@jay jay added the tests label Apr 7, 2020
Copy link
Member

MarcelRaad left a comment

👍
Why didn't I just to it this way in #2786? I think you can also remove the chkdecimalpoint stuff now.

@bagder
bagder approved these changes Apr 7, 2020
@bagder
bagder approved these changes Apr 8, 2020
@bagder
Copy link
Member

bagder commented Apr 8, 2020

Hm, I could apparently approve it twice! 😁

@jay jay closed this in 17c18fb Apr 11, 2020
@jay jay deleted the jay:fix_test1148 branch Apr 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.