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

Conversation

@rob05c
Copy link
Member

@rob05c rob05c commented Nov 1, 2022

Moves the TO test setup to a shared library to remove duplication in the ORT/t3c integration tests, because that was easier than upgrading t3c tests to API v4, which was necessary to test multiple profiles.

It turned out, moving the TO test data setup to a library was easier than fixing all the changes in v4.

It's a lot of lines changes (3k duplicate lines removed), but there's no logic change in the tests, just moving and renaming things. More conveniences could be added, like moving the WithObjs helpers to the library and removing the wrappers. But it seemed better to keep this as small as possible; it's already huge.

Which Traffic Control components are affected by this PR?

  • Traffic Control Cache Config (t3c, formerly ORT)
  • Traffic Control Client Go

What is the best way to verify this PR?

Run tests.

If this is a bugfix, which Traffic Control versions contained the bug?

Not a bug fix.

PR submission checklist

  • This PR has tests
  • [x] This PR has documentation no docs, no interface change
  • [x] This PR has a CHANGELOG.md entry no changelog, no user interface change
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

@rob05c rob05c mentioned this pull request Nov 1, 2022
2 tasks
@zrhoffman zrhoffman added tech debt rework due to choosing easy/limited solution cache-config Cache config generation TO Client (Go) related to the Go implementation of a TC client high impact impacts the basic function, deployment, or operation of a CDN improvement The functionality exists but it could be improved in some way. labels Nov 1, 2022
@rob05c rob05c force-pushed the fix-t3c-integration-test-duplication branch 13 times, most recently from 24f1eb3 to 109eee3 Compare November 3, 2022 19:38
@rob05c rob05c requested a review from zrhoffman November 3, 2022 21:41
@rob05c
Copy link
Member Author

rob05c commented Nov 3, 2022

It'd be really good to do this to v5 as well, so the t3c Integration Tests can easily share that too, when it comes time to upgrade them.
But that can be done separately.

@rob05c rob05c force-pushed the fix-t3c-integration-test-duplication branch from 109eee3 to 274dfc8 Compare November 3, 2022 21:50
Moves the TO test setup to a shared library to remove duplication
in the ORT/t3c integration tests, because that was easier than
upgrading t3c tests to API v4, which was necessary to test multiple
profiles.
@rob05c rob05c force-pushed the fix-t3c-integration-test-duplication branch from 274dfc8 to 20527e4 Compare November 7, 2022 15:58
@traeak
Copy link
Contributor

traeak commented Dec 8, 2022

Testing:
Check out rob's branch, create t3c pre PR and t3c post PR.
Run against same host and compare files.

Sample commands and then compare:
for hdr_rw in hdr_rw* ; do echo $hdr_rw ; grep -v '^#' ${hdr_rw} | md5sum ; done > ~/hdr_rw_sums.txt; md5sum ~/hdr_rw_sums.txt
grep -v '^#' parent.config | md5sum
grep -v '^#' records.config | md5sum

Copy link
Contributor

@traeak traeak left a comment

Choose a reason for hiding this comment

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

Generated files with previous git has and with this git has and compared for multiple configurations and tiers in our prod. File contents were the same.

@ericholguin
Copy link
Contributor

This was included in #7287

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cache-config Cache config generation high impact impacts the basic function, deployment, or operation of a CDN improvement The functionality exists but it could be improved in some way. tech debt rework due to choosing easy/limited solution TO Client (Go) related to the Go implementation of a TC client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants