Skip to content

skpkg: setup CI after migrating src, tests, requirements, and .github directory #37

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

Merged
merged 9 commits into from
Jun 28, 2025

Conversation

dabeycorn
Copy link

No description provided.

dabeycorn and others added 9 commits June 28, 2025 10:26
Signed-off-by: Dasun Abeykoon <Dasun20202020@gmail.com>
Signed-off-by: Dasun Abeykoon <Dasun20202020@gmail.com>
Signed-off-by: Dasun Abeykoon <Dasun20202020@gmail.com>
Signed-off-by: Dasun Abeykoon <Dasun20202020@gmail.com>
Signed-off-by: Dasun Abeykoon <Dasun20202020@gmail.com>
Signed-off-by: Dasun Abeykoon <Dasun20202020@gmail.com>
Signed-off-by: Dasun Abeykoon <Dasun20202020@gmail.com>
Signed-off-by: Dasun Abeykoon <Dasun20202020@gmail.com>
@dabeycorn
Copy link
Author

dabeycorn commented Jun 28, 2025

@sbillinge @zmx27, there are some problems with the tests. After checking the pre-migration code, I found that the tests were failing before I started the migration process. I also noticed that the tests appear to be headless even though the older tests-on-pr workflow marked them as non-headless. I'm not exactly sure how to mitigate these issues. Could you please advise?

@sbillinge
Copy link
Contributor

@sbillinge @zmx27, there are some problems with the tests. After checking the pre-migration code, I found that the tests were failing before I started the migration process. I also noticed that the tests appear to be headless even though the older tests-on-pr workflow marked them as non-headless. I'm not exactly sure how to mitigate these issues. Could you please advise?

ok, I will take a look.

@sbillinge
Copy link
Contributor

@dabeycorn the changes all look good so I will merge this with tests failing so we have all these linting changes.

@sbillinge sbillinge merged commit e01a192 into diffpy:migration Jun 28, 2025
1 of 2 checks passed
@sbillinge
Copy link
Contributor

@dabeycorn the test failings are legit. I don't think it needs to run headless tbh, but we have to fix the tests. Let's work on that in the next PR. Please sync the migration branch and we can work on that. Have a look at it yourself and see if you can figure it out. I can help with comments in any resulting PR. If we can't figure it out, I can try and pull it locally and work on it, but see if you can make progress with @zmx27 first.

@dabeycorn
Copy link
Author

Sounds good, I'll take a look at the failing tests

@zmx27
Copy link

zmx27 commented Jun 28, 2025

@dabeycorn After cloning the repo locally and taking a look at the errors, I believe that the pytest errors can all be resolved if you take a look at the error messages that it gives you and fix the discrepancies in the assertion statements. Let me know if you need any further clarifications.

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.

3 participants