Skip to content

Conversation

@Claudenw
Copy link
Contributor

Thanks for sending a pull request! Here are some tips if you're new here:

  • Ensure you have added or run the appropriate tests for your PR.
  • Be sure to keep the PR description updated to reflect all changes.
  • Write your PR title to summarize what this PR proposes.
  • If possible, provide a concise example to reproduce the issue for a faster review.
  • Read our contributor guidelines
  • If you're making a documentation change, see our guide to documentation contribution

Commit messages should follow the following format:

<One sentence description, usually Jira title or CHANGES.txt summary>

<Optional lengthier description (context on patch)>

patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####

Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>

The Cassandra Jira

@Claudenw
Copy link
Contributor Author

Claudenw commented Apr 2, 2024

@michaelsembwever, If you recall I submitted a pull request for Cassandra-12937 that included a change to the topology.properties file. You asked why that change was made and I could not answer as I received it from a development branch that I did not work on. However, I discovered that if you run ant test that file would be overwritten. While this is not a problem on the CI system it does cause a problem on development systems where the modified file can be accidentally checked in 🖐️. This change to one file solves the problem. Please review and let's see if we can get this in.

localAddress = FBUtilities.getBroadcastAddressAndPort();

restoreOrigConfigFile();
Files.copy(effectiveFile, backupFile, java.nio.file.StandardCopyOption.REPLACE_EXISTING);
Copy link
Contributor

Choose a reason for hiding this comment

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

@After should work as well, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not for the Files.copy but for the finally of the try/catch that calls restoreOrigConfigFile(). In fact restoreOrigConfigFile could be annotated with @After

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is the right way to go :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated pull request after testing to ensure that it works as expected.

@Mmuzaf
Copy link
Contributor

Mmuzaf commented Apr 3, 2024

I can confirm that the problem is reproducible without the changes, and is fixed with the changes.

The command to reproduce:
ant test (the scope of the files must be narrowed in advance)

However, the command below doesn't reproduce the issue:
ant testclasslist -Dtest.classlistfile=testnameslist.txt

@Mmuzaf Mmuzaf closed this Apr 3, 2024
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.

2 participants