Skip to content

Conversation

@woodsh17
Copy link
Member

@woodsh17 woodsh17 commented Oct 10, 2025

This PR updates the tests so that all the examples run and if an example fails then the test results in a failure as well. Changes include:

  • Reformatting design_macrocyclic_binder.sh and design_macrocyclic_monomer.sh to be submitted correctly by test_diffusion.py
  • Reducing the total length in design_tetrahedral_oligos.sh to reduce run time of this test
  • Changes to test_diffusion.py and main.yml to be able to run the examples in different chunks so examples can run in parallel and to make sure that if an example errors out, that the tests does not pass.

Currently design_ppi_scaffolded, design_timbarrel, and design_ppi_flexible_peptide_with_secondarystructure_specification are failing which should be addressed in other future PRs.

@woodsh17 woodsh17 marked this pull request as ready for review October 23, 2025 17:33
@woodsh17 woodsh17 requested review from lyskov and rclune October 23, 2025 18:38
Copy link
Member

@rclune rclune left a comment

Choose a reason for hiding this comment

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

Thanks for reformatting so much stuff to make it more readable!

Copy link
Member

@rclune rclune left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@lyskov lyskov left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for taking care of of this @woodsh17 !

TestSubmissionCommands.total_chunks = args.total_chunks
TestSubmissionCommands.chunk_index = args.chunk_index

unittest.main(argv=[sys.argv[0]] + remaining)
Copy link
Member

Choose a reason for hiding this comment

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

@woodsh17 I am assuming that this was simply reformatted with PEP8 settings, - is that so?

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you referencing changing unittest.main() to unittest.main(argv=[sys.argv[0]] + remaining)? These arguments were added so we don't pass the custom arguments --total_chunks and --chunk_index to unittest, which would be unrecognized.

else
echo "A chunk (PID $pid) passed"
fi
done
Copy link
Member

Choose a reason for hiding this comment

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

LGTM! I would personally would start to consider moving this logic into Python/shell script that live inside repository so it could be run/tested without the need to trigger the GH action. But for simple cases (like above) current approach is also works. 👍

After running the test, I found that this was needed
This reverts commit ead721f.
Copy link
Member

@rclune rclune left a comment

Choose a reason for hiding this comment

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

LGTM

@woodsh17 woodsh17 merged commit ff20fba into main Nov 13, 2025
1 of 2 checks passed
@woodsh17 woodsh17 deleted the fix_test_diffusion branch November 13, 2025 21:33
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.

4 participants