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.

Sign up for GitHub

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

Jump to bottom

bin: add scripts for running tests locally #167

Merged
merged 8 commits into from
Apr 17, 2024
Merged

bin: add scripts for running tests locally #167

merged 8 commits into from
Apr 17, 2024

Conversation

ErikSchierboom
Copy link
Member

@ErikSchierboom ErikSchierboom commented Mar 7, 2024 •
edited
Loading

Closes #163

@ErikSchierboom ErikSchierboom requested a review from a team as a code owner March 7, 2024 08:08
@ee7 ee7 changed the title Allow tests to run locally bin: add scripts for running tests locally Mar 8, 2024
ee7
ee7 requested changes Mar 8, 2024
View reviewed changes
Copy link
Member

@ee7 ee7 left a comment

Choose a reason for hiding this comment

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

Looks like these don't quite work as-is, so we shouldn't merge this yet.

@ErikSchierboom
Copy link
Member Author

Looks like these don't quite work as-is, so we shouldn't merge this yet.

Looks like the expected_results.json files are nicely formatted, whereas the results.json files aren't. Normally I'd use something like jq to fix this, but as this runs within the test runner image (which doesn't have jq), this is not an option. Thoughts?

@ErikSchierboom
Copy link
Member Author

@ee7 small bump

@ee7
Copy link
Member

ee7 commented Apr 3, 2024

Let's just add jq to the Dockerfile? Seems like the quickest solution for now.

@ErikSchierboom ErikSchierboom force-pushed the allow-local-running-of-tests branch from 48d710f to 8c90124 Compare April 3, 2024 09:38
@ErikSchierboom ErikSchierboom force-pushed the allow-local-running-of-tests branch from f17738d to d182ebc Compare April 5, 2024 09:11
ErikSchierboom
ErikSchierboom commented Apr 5, 2024
View reviewed changes
tests/error/compiletime_crash_in_solution/expected_results.json Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

I've formatted the expected_results.json files using jq

tests/error/compiletime_error_closing_quote_expected/test_identity.nim
@@ -1,5 +1,5 @@
import std/unittest
import hello_world
import identity
Copy link
Member Author

Choose a reason for hiding this comment

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

The run-tests.sh script passes in the exercise slug, and all test cases use "identity" except for this one and a couple of others. I could have added some logic to the test runner to figure out the slug, or just renamed it (which is what I did).

@ErikSchierboom ErikSchierboom requested a review from ee7 April 5, 2024 09:51
@ErikSchierboom ErikSchierboom merged commit dc5448b into main Apr 17, 2024
8 checks passed
@ErikSchierboom ErikSchierboom deleted the allow-local-running-of-tests branch April 17, 2024 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ee7 ee7 Awaiting requested review from ee7

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow running smoke tests locally
2 participants
@ErikSchierboom @ee7