#903: added vet flag to resolve race issue

[vanshaj]

As per the last comment in issue : golang/go#27089
I have turned off the vet flag and "make tests" worked fine on my windows machine.

Kindly review

[olegbespalov]

Hi @vanshaj !

Thanks for your PR.

Could you please provide more details about the race issue? Maybe an output? 🤔

Cheers!

[vanshaj]

Hi @vanshaj !

Thanks for your PR.

Could you please provide more details about the race issue? Maybe an output? 🤔

Cheers!

yess, Kindly have a look at below image

[olegbespalov]

In #903, I believe @mstoykov meant, first of all, the CI, but we don't run make test there. And it seems like in one of the last CI refactorings. We fixed it with the workaroubnd.

However, I believe it's also essential to have a make test target fixed too (maybe at some point, both the local & CI will run the same command). So let's try to finalize this PR 👍

[olegbespalov]

I see that, for some reason, the change affected whole formatting. Is it possible to keep only the actual change(s)?

[vanshaj]

Sure I will make the formatting correct.

[olegbespalov]

Hi @vanshaj

It seems like the whole file formatting is still affected. It shows that 30 lines were removed and 30 added:

[olegbespalov]

Just a small note here that this affects runs for all operation systems which isn't good, but I guess in that case it's fine as long as we have the linter altogether (make check)

[vanshaj]

@olegbespalov Do you want me to add some check related to underlying OS?

[olegbespalov]

No, I guess in the current case, it will be over. Just share the opinion that in some cases we can do that.

[codecov-commenter]

Codecov Report

Merging #2595 (cbad21c) into master (7f823f0) will decrease coverage by 0.23%.
The diff coverage is 66.83%.

@@            Coverage Diff             @@
##           master    #2595      +/-   ##
==========================================
- Coverage   75.59%   75.36%   -0.24%     
==========================================
  Files         202      205       +3     
  Lines       15992    16287     +295     
==========================================
+ Hits        12089    12274     +185     
- Misses       3151     3248      +97     
- Partials      752      765      +13     

Flag	Coverage Δ
ubuntu	75.20% <66.83%> (-0.19%)	⬇️
windows	75.05% <66.83%> (-0.24%)	⬇️

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

Impacted Files	Coverage Δ
errext/exit_code.go	100.00% <ø> (ø)
js/common/interrupt_error.go	100.00% <ø> (+18.18%)	⬆️
js/compiler/compiler.go	63.52% <ø> (-0.63%)	⬇️
js/modules/k6/grpc/client.go	78.39% <0.00%> (ø)
js/timeout_error.go	56.25% <ø> (ø)
lib/consts/consts.go	78.57% <ø> (ø)
metrics/metric.go	92.40% <0.00%> (+3.38%)	⬆️
output/csv/time_format_gen.go	20.00% <20.00%> (ø)
metrics/thresholds.go	88.54% <33.33%> (-0.18%)	⬇️
...utils/httpmultibin/grpc_any_testing/any_test.pb.go	37.97% <37.97%> (ø)
... and 49 more

---

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 bfa415c...cbad21c. Read the comment docs.

[olegbespalov]

Hey @oleiade. Could you please share your thoughts about the PR? Thanks.

[oleiade]

Once again, thanks for your contribution @vanshaj, we really appreciate and value that you took the time and put in the effort to propose an improvement to k6.

Unfortunately, we discussed it internally, and experimented around this issue internally, and came to the conclusion we would not merge this PR in master. The main reason being, we cannot reproduce the issue you reported, and as a result, we can't risk introducing a change that might harm other users (including ourselves) in ways we cannot foresee for a potential benefit we cannot observe.

I hope this makes sense, and that you understand. Please don't hesitate to contribute again in the future 🙇🏻