nealrichardson / httptest

A Test Environment for HTTP Requests in R

https://enpiar.com/r/httptest/

Other

79 stars 10 forks source link

Fix CRLF issue on GHA #37

Closed nealrichardson closed 3 years ago

nealrichardson commented 3 years ago

I've gotten rid of the \r\n mess and the strings look visually the same but for some reason they're still not testing as identical. Will have to debug further.

FYI @jonkeane

codecov-io commented 3 years ago

Codecov Report

Merging #37 into master will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #37   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines          430       435    +5     
=========================================
+ Hits           430       435    +5

Impacted Files	Coverage Δ
R/capture-requests.R	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update eda0167...0926244. Read the comment docs.

jonkeane commented 3 years ago

Welcome to my world! 🙃 I did a bunch of feeling around in the dark to find a difference, I even tried the large hammer of gsub("[ \t\r\n\\h\\v]", "", content(...)) and still there was some mysterious difference that made the strings different.

nealrichardson commented 3 years ago

Doing some spelunking with waldo and cleaning up the output, I now see

old[168:174] vs new[168:174]
  "                                //operationsSorter: \\"alpha\\","
  "                                presets: ["
  "                                    SwaggerUIBundle.presets.apis,"
- "                                    // yay ES6 modules <U+2198>"
+ "                                    // yay ES6 modules <U+2198>"
  "                                    Array.isArray(SwaggerUIStandalonePreset) ? SwaggerUIStandalonePreset : SwaggerUIStandalonePreset.default"
  "                                ],"
  "                                plugins: ["

which confirms that somehow this is a windows unicode issue (and extra annoying that this is choking on (1) a code comment (2) in javascript (3) with a gratuitous special character (4) to express excitement about some ES6 hotness).

nealrichardson commented 3 years ago

Fixed! PTAL @jonkeane. Not sure if all of the UTF-8 encoding everywhere is necessary but it shouldn't hurt. Doing the string comparison after converting to native encoding though was the change that made the test pass in the end (after switching to a binary file connection to stop the CRLF conversion).

jonkeane commented 3 years ago

Looks good to me (and looks like you also ran into curl not installing for 3.3 on ubuntu as well? I hard coded it, but this is definitely better)

nealrichardson commented 3 years ago

Looks good to me (and looks like you also ran into curl not installing for 3.3 on ubuntu as well? I hard coded it, but this is definitely better)

yeah, I'm guessing this is in the r-lib/actions workflow template because you added it when you added GHA, but I deleted it (foolishly) because hey, httptest doesn't have system dependencies 🤦