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

Fix: local checkfile used in testing is now general enough #69

Merged
merged 17 commits into from
Oct 26, 2023

Conversation

simojo
Copy link
Collaborator

@simojo simojo commented Sep 22, 2023

Previously, the local checkfile .chasten/checks.yml had checks searching for things like doubly-nested if statements found in the chasten source code. Because finding things like this in our repository source code is not set in stone, I have changed this to be general enough to effectively test the functionality without making false assumptions such as the one previously mentioned.

Closes #59

NOTE: depends on #68 before merging.

NOTE: @gkapfham is pointing out that #68 has now been merged.

@simojo simojo changed the title Fix: local checkfile used in testing is now general enough WIP: Fix: local checkfile used in testing is now general enough Sep 22, 2023
@simojo simojo added infrastructure Docker and CI/CD setup and/or configuration ready-for-review This pull request is ready for review labels Sep 22, 2023
@simojo simojo self-assigned this Sep 22, 2023
@simojo simojo force-pushed the fix/local-checkfile branch from 8af0c93 to d12e0b7 Compare September 22, 2023 15:04
these checks were hindering our build. They serve no semantic purpose, so it is completely appropriate to remove them.
@simojo
Copy link
Collaborator Author

simojo commented Oct 18, 2023

It appears that we're having problems with coverage here. I'll continue to look into this and comment what I find here. I was not expecting this. @laurennevill

Hopefully fixes the pip install issue on windows
@laurennevill
Copy link
Collaborator

Hey @gkapfham, this is priority one for merges

@simojo
Copy link
Collaborator Author

simojo commented Oct 20, 2023

The problem with coverage is still unidentified and unsolved, just letting this thread know.

@boulais01 boulais01 requested a review from gkapfham October 25, 2023 13:48
@simojo
Copy link
Collaborator Author

simojo commented Oct 25, 2023

@laurennevill @gkapfham , Mordred and I found that the failing checks is a transient issue. I suspect that, because coverage is time consuming, the process is timing out, but this does not appear to be obvious anywhere. Regardless, we have proven that the issue is not consistent, and we both recommend merging this when possible.

Copy link
Collaborator

@laurennevill laurennevill left a comment

Choose a reason for hiding this comment

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

Good job working so hard on this over the past few weeks. Hopefully this will be one step closer to a better build!

@simojo simojo changed the title WIP: Fix: local checkfile used in testing is now general enough Fix: local checkfile used in testing is now general enough Oct 25, 2023
@gkapfham gkapfham merged commit 6ade991 into AstuteSource:master Oct 26, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Docker and CI/CD setup and/or configuration ready-for-review This pull request is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test case that uses source code of chasten fails in development branch
4 participants