Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove 'make test' logic from .github/workflows #343

Merged
merged 3 commits into from
Jan 14, 2025
Merged

Conversation

martinholmer
Copy link
Collaborator

Fixes #338 by not running tests on GitHub.

The "make test" command still works locally when the two 2015 PUF files are located in the tmd/storage/input folder.

@martinholmer martinholmer marked this pull request as draft January 14, 2025 00:43
@martinholmer martinholmer marked this pull request as ready for review January 14, 2025 00:53
Copy link
Collaborator

@donboyd5 donboyd5 left a comment

Choose a reason for hiding this comment

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

Thanks, @martinholmer, and FYI @nikhilwoodruff. I think this is a good solution. I prefer it to a working version of PR #342 because I don't believe we can be confident of (and have time for) creating synthetic data that would be equivalent for test purposes to the real data.

In addition, it solves a potentially concerning issue with the existing PAT approach: in that approach we effectively are releasing the PUF to anyone who has a PAT, perhaps putting us into a policing role we don't want. (The synthetic data approach solves this issue, too.)

I know you don't need approval for merging, but I recommend you go forward with merging.

I do have one reservation that we might want to talk about if we end up doing follow-on work on tmd. If there ever are many contributors to this project, we might want some mechanism to enforce test-passing before changes are merged. As I understand it, this PR puts the onus on the contributors (in practice, only you and me now) to be sure tests pass on our local machines before we merge anything. That's probably fine with just 2 of us, but if more people become involved it seems not safe enough, plus it doesn't ensure that tests pass on multiple machines. So if we work on this again, I suggest we discuss this.

@martinholmer martinholmer merged commit 6a49c04 into master Jan 14, 2025
1 check passed
@martinholmer martinholmer deleted the rm-gh-wf-test branch January 14, 2025 14:46
@martinholmer
Copy link
Collaborator Author

martinholmer commented Jan 14, 2025

@donboyd5, Thanks for your thoughtful comments on PR #343.

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.

The "make install" command fails
2 participants