Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Traffic Ops unit tests: TestGetMonitoringJSON sometimes fails#4920

Merged
ocket8888 merged 3 commits intoapache:masterfrom
srijeet0406:CDN-10229
Jul 30, 2020
Merged

Traffic Ops unit tests: TestGetMonitoringJSON sometimes fails#4920
ocket8888 merged 3 commits intoapache:masterfrom
srijeet0406:CDN-10229

Conversation

@srijeet0406
Copy link
Contributor

@srijeet0406 srijeet0406 commented Jul 29, 2020

What does this PR (Pull Request) do?

Which Traffic Control components are affected by this PR?

  • Traffic Ops

What is the best way to verify this PR?

Follow the steps in #4919

cd traffic_ops/traffic_ops_golang/monitoring
go test -c
failures=0
for ((i = 0; i < 100; i++)); do
  ./monitoring.test || (( ++failures ))
done;
echo "Failed ${failures} times out of ${i}"

should result in 0 failures, always.

If this is a bug fix, what versions of Traffic Control are affected?

  • master

The following criteria are ALL met by this PR

  • This PR includes tests
  • I have explained why documentation is unnecessary
  • This PR does not include an update to CHANGELOG.md
  • This PR includes any and all required license headers
  • This PR does not include a database migration
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@srijeet0406 srijeet0406 changed the title Fixing GetMonitoringJson test Traffic Ops unit tests: TestGetMonitoringJSON sometimes fails Jul 29, 2020
Copy link
Member

@zrhoffman zrhoffman left a comment

Choose a reason for hiding this comment

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

Although the tests do pass, is there a way to accomplish this that makes use of the existing code? Take a look at the sortable types in this file and how they implement interface.Less() and interface.Swap() so they can use sort.Sort().

@srijeet0406
Copy link
Contributor Author

@zrhoffman I changed it so that it gets called from the sortCaches method now. The servers sorting is different, in the sense that we need to double sort the slice, once by the server hostname, and the second sorting will be by the interface names of each server.

@zrhoffman
Copy link
Member

zrhoffman commented Jul 30, 2020

The servers sorting is different, in the sense that we need to double sort the slice, once by the server hostname, and the second sorting will be by the interface names of each server

Right, but for the interface sorting part, is there a way to use a SortableInterfaces type and sort on that?

If we can use the same code pattern for sorting across different Go interfaces, it will make it easier for the next maintainer who has to modify this code.

@srijeet0406
Copy link
Contributor Author

@zrhoffman Should be done now

Copy link
Member

@zrhoffman zrhoffman left a comment

Choose a reason for hiding this comment

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

Awesome, looks great and passes consistently.

@ocket8888 ocket8888 merged commit 7691d16 into apache:master Jul 30, 2020
@mitchell852 mitchell852 added Traffic Ops related to Traffic Ops tests related to tests and/or testing infrastructure labels Aug 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tests related to tests and/or testing infrastructure Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Traffic Ops unit tests: TestGetMonitoringJSON sometimes fails

4 participants