PIE-1478/Remove identifier from trace

[amybiyuliu]

We no longer using identifier in TA and have stopped saving the value in our database. This PR removes identifier from Rspec trace and Minitest trace. We are not going to make a release for this small change.

Ticket: https://linear.app/buildkite/issue/PIE-1478/remove-identifier-from-ruby-test-collector Parent ticket: https://linear.app/buildkite/issue/PIE-1427/remove-identifier-from-test-collector-payloads

To verify the change, Rose and I paired together to do an integration test for Rspec.I updated the gemfile on my local buildkite repo to use the local test-collector-ruby gem

• Running rspec test at local not causing any errors
• Records are created in the database, and we can see the runs and executions from the UI
• identifier is not included in the S3 file

To verify the changes to the minitest collector, I tested the changes against this PR https://github.com/buildkite/buildkite/pull/11383.

• Running minitest at local not causing any errors
• Records are created in the database, and we can see the runs and executions from the UI
• identifier is not included in the S3 file

[niceking]

Looks good @amybiyuliu!

In the minitest trace, it looks like identifier is being aliased to the location method: https://github.com/buildkite/test-collector-ruby/blob/2715713f506977d7e5871e1a6e3ed58b25fa23c8/lib/buildkite/test_collector/minitest_plugin/trace.rb#L54

This can be removed as part of this change too :)

This change looks correct, but I just wanted to nitpick on this part of your description of testing:

To verify the change, I made the same changes to the gem at local and it worked well when I run the Rspec tests.

As a reviewer, it would be helpful here to understand what you verified as part of your testing! I think for clarity it would be better to be a bit more specific than "it worked well". The things I would be looking for to verify this change would be:

• no observable errors when running tests with local changes to the test collector
• execution records created
• S3 upload created from your test doesn't include identifier anymore
• works for rspec and minitest test harnesses

All good if you have done all that testing, and that's what you meant by "works well" and you can just change the description to reflect that 😄 Also happy to pair on any bits of testing this if you're unsure on how to do any of the test verification I mentioned! I haven't actually gotten around to making a setup to run minitest tests locally yet so maybe that's something we can do together!

[niceking]

Also, as this change is pretty minor, I would probably not create a separate release for just this change. There'll be a bunch of upcoming collector changes, and this can be rolled in with one of them. Happy to also pair on the gem release process 😄

[amybiyuliu]

@niceking I was just about to ask you if you have a PR for the minitests lol. I'll verify the changes using your PR. Thanks a million!