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

Update other dependencies #1940

Merged
merged 10 commits into from
Apr 1, 2021
Merged

Update other dependencies #1940

merged 10 commits into from
Apr 1, 2021

Conversation

mstoykov
Copy link
Contributor

@mstoykov mstoykov commented Mar 31, 2021

This updates the majority of the k6 dependencies that we intended to update from #1933.

The biggest size increases are from yaml.v3 addition, compression algorithms, and testify additions.

This has some fixes for unmarshalling which we barely use, it is mostly
done so we are up to date.
…1.3.0

There are some bugfixes including some panics in rare cases that are now
fixed.
…eb68cc467c

There are a lot of optimizations especially in less allocations.

Also it drops a dependency which means this is actually less code in the
k6 repo 🎉
no tagged releases but we go from commit from the year 2015 to one from 2019

Added an additional method to get number of pooled buffers and ByteSlicePool
The only change is a change to it's CI
This mostly add logging through functions and drops a dependancy by just
embeding the relevant parts in logrus.
This adds some additional helpers most interesting of which support for
the new errors and friends.

This also adds v3 of gopkg.in/yaml as a dependency but this likely will
happen from some other dependency either way
There are some additional syntax supports.

The update github.com/tidwall/pretty is used in `convert/har` and stopped
adding spaces after commas in multi line arrays which required the
changes to `testdata/example.js`
@mstoykov mstoykov requested review from imiric and na-- March 31, 2021 11:10
@mstoykov
Copy link
Contributor Author

Please go commit by commit as they explain what is changed and I prefer to not copy it ;)

@mstoykov mstoykov added this to the v0.32.0 milestone Mar 31, 2021
@codecov-io
Copy link

codecov-io commented Mar 31, 2021

Codecov Report

Merging #1940 (0a5f343) into master (6f2b9b0) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 0a5f343 differs from pull request most recent head eaff1e6. Consider uploading reports for the commit eaff1e6 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1940   +/-   ##
=======================================
  Coverage   71.23%   71.24%           
=======================================
  Files         183      183           
  Lines       14336    14336           
=======================================
+ Hits        10212    10213    +1     
  Misses       3484     3484           
+ Partials      640      639    -1     
Flag Coverage Δ
ubuntu 71.20% <ø> (+<0.01%) ⬆️
windows 70.90% <ø> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/testutils/minirunner/minirunner.go 76.08% <0.00%> (-4.35%) ⬇️
lib/executor/vu_handle.go 95.23% <0.00%> (+0.95%) ⬆️
lib/testutils/httpmultibin/httpmultibin.go 91.01% <0.00%> (+1.12%) ⬆️

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 6f2b9b0...eaff1e6. Read the comment docs.

Adds an additional method ProcessChallengeWithHash which gets a hash
instead of user and password.
…v1.7.2

This is nearly 2 years of updates, most of which seem to be performance
related.
@mstoykov
Copy link
Contributor Author

The only remaining dependencies that should be updated at all from #1933 are:

  • github.com/DataDog/datadog-go there were a lot of changes including new dependencies for support of windows pipes and some aggregation code. Given that it still doesn't support tcp and we might want to add some support for the aggregation I decided I should look at it more closely and open an issue to discuss what we should do.
  • github.com/fatih/color, github.com/mattn/go-colorable and github.com/mattn/go-isatty - there are a lot of changes here including a lot of fixes for windows (supposedly). I did test it under Linux somewhat, but decided that given the amount of changes a lot more testing under windows will be needed and decided that would mean it should be in a separate PR
  • github.com/spf13/cobra and github.com/spf13/pflag - again a lot more changes including more support for context and some windows related changes. So it is actually a combination of the two above - I want to look at the changes in more detail and it likely will require more testing under windows. Also did update color libraries somewhat which means that it should probably happen after we update those ;)

There were no updates for:

  • github.com/nu7hatch/gouuid
  • github.com/mccutchen/go-httpbin

Copy link
Member

@na-- na-- left a comment

Choose a reason for hiding this comment

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

This PR adds gopkg.in/yaml.v3, while keeping gopkg.in/yaml.v2. I think we can drop the latter with a minor change in k6:

$ go mod why 'gopkg.in/yaml.v3'
# gopkg.in/yaml.v3
github.com/loadimpact/k6/lib/testutils/httpmultibin
github.com/stretchr/testify/assert
gopkg.in/yaml.v3
$ go mod why 'gopkg.in/yaml.v2'
# gopkg.in/yaml.v2
github.com/loadimpact/k6/ui
gopkg.in/yaml.v2

@mstoykov
Copy link
Contributor Author

mstoykov commented Apr 1, 2021

Unfortunately - not @na-- , as it's also used in gin and another yaml library which used by helm of which we use strvals a yes and highlighter

Also even if we could I prefer to do it in a separate PR, where I will drop the direct uses by k6

and also the grpc library use the yaml.v2 so 🤷

Copy link
Member

@na-- na-- left a comment

Choose a reason for hiding this comment

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

golang/go#41305 ... 😞

Copy link
Contributor

@imiric imiric left a comment

Choose a reason for hiding this comment

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

🤞 A lot of new LOCs, but it seems unavoidable.

Nice job reviewing each update and detailing it in separate commits! 👏

@mstoykov mstoykov merged commit 259982a into master Apr 1, 2021
@mstoykov mstoykov deleted the updateOtherDependencies branch April 1, 2021 08:39
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.

4 participants