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

Avoid test keyword #44

Merged
merged 2 commits into from
Jan 15, 2024
Merged

Avoid test keyword #44

merged 2 commits into from
Jan 15, 2024

Conversation

kljensen
Copy link
Contributor

This PR replaces test in the code because it is reserved for future use in recent Gleam versions.

I'd appreciate some feedback. I realize that variable name the_test stinks. Happy to change.

@kljensen kljensen requested a review from a team as a code owner January 12, 2024 20:03
Copy link

@giacomocavalieri giacomocavalieri left a comment

Choose a reason for hiding this comment

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

Thank you! There is just a small change that needs to be reverted

runner/src/exercism_test_runner/internal.gleam Outdated Show resolved Hide resolved
@kljensen
Copy link
Contributor Author

LOL - that is super embarrassing sorry! I couldn't run the tests locally (process didn't finish running tests in docker) and wanted to see the tests run in the PR on GitHub Actions...hence the PR w/o seeing tests.

But, having said that is it super weird that the tests ran without error given this obvious...error.

@giacomocavalieri
Copy link

Yeah that's an interesting behaviour for sure! I'll have to look into that :)

@kljensen
Copy link
Contributor Author

kljensen commented Jan 13, 2024

Yeah that's an interesting behaviour for sure! I'll have to look into that :)

@giacomocavalieri it looks like the tests in the runner directory are not run in CI, but those are the tests that count. Notice that the tests in the tests directory have pinned versions of this package. The effect is, I believe, that the run-tests-in-docker.sh script that is run in CI actually doesn't test the code in a PR.

One solution to this might be to use something like

[dev-dependencies]
exercism_test_runner = { path = "../../runner" }

But...well, I'm new to gleam and having a tad of trouble with that when I gave it a half-hearted try.

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Thank you

@lpil lpil merged commit a998225 into exercism:main Jan 15, 2024
1 check passed
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