Skip to content

Conversation

@ee7
Copy link
Member

@ee7 ee7 commented Jan 13, 2023

Before this commit, the user-facing solution file was empty for every exercise apart from:

  • clock
  • dnd-character
  • hello-world
  • react

Add initial content to the user-facing solution file for every other exercise.

And check in CI that every exercise has a test file and stub that:

  • compiles without error
  • runs with an error (we want the stub to fail the tests)

Unfortunately, for now, this significantly increases the time taken to run CI (for the fastest exercises job: from about 25 seconds to about 4 minutes).

Closes: #409
Closes: #410


To-do:

Some things left to consider:

  • Rename lasagna parameters to match the docs
  • Table vs CountTable vs array
  • Remove range types?
  • Parameters that want to have default values
  • Parameter names
  • Ways to speed up CI
    • actions/cache
    • Incremental compilation
    • Only run check_stubs.nim when one of these files changes: check_stubs.nim itself, an exercise stub, or an exercise test file
    • Make check_stubs.nim run only on Linux
    • Make check_stubs.nim run via a separate workflow

Policies:

  • use proc, not func
  • (todo)

For later PRs:

  • Consider doc comments

@ee7
Copy link
Member Author

ee7 commented Jan 14, 2023

CI log when stubs passed checks:

Checking stubs...
lasagna
acronym
all-your-base
[...]
two-fer
word-count
yacht

Success. Every exercise has a test file and stub that:
- compiles without error
- runs with an error (we want the stub to fail the tests)

Example CI log when bad stubs failed checks:

Checking stubs...
lasagna
test_lasagna.nim(4, 7) template/generic instantiation of `suite` from here
test_lasagna.nim(13, 8) template/generic instantiation of `test` from here
test_lasagna.nim(14, 5) template/generic instantiation of `check` from here
test_lasagna.nim(15, 7) Error: undeclared identifier: 'preparationTimeInMinutes'

acronym
[...]
collatz-conjecture
collatz_conjecture.nim(2, 1) Error: invalid indentation

crypto-square
[...]
gigasecond
gigasecond.nim(1, 25) Error: undeclared identifier: 'DateTime'

grade-school
grains
hamming
hello-world

[Suite] Hello World
  [OK] say hi!
Error: the hello-world stub passed the tests

high-scores
[...]
two-fer
test_two_fer.nim(4, 7) template/generic instantiation of `suite` from here
test_two_fer.nim(5, 8) template/generic instantiation of `test` from here
test_two_fer.nim(6, 11) Error: undeclared identifier: 'twoFer'

word-count
yacht

Error: there were 5 exercises with a problematic stub:
lasagna, collatz-conjecture, gigasecond, hello-world, two-fer

@ee7
Copy link
Member Author

ee7 commented Mar 1, 2023

I was hoping to finish this a long time ago. Now that it's Mechanical March already, I'd like to merge this soon. Requested reviewers: could either of you check that the stubs look reasonable? I'll keep this as draft until I've fixed the names in lasagna.

CI does check that every exercise has a test file and stub that:

  • compiles without error
  • runs with an error (we want the stub to fail the tests)

@ee7 ee7 requested review from ErikSchierboom and ynfle March 1, 2023 13:14
Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

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

Fabulous!

ee7 added 3 commits March 5, 2023 12:19
Before this commit, the user-facing solution file was empty for every
exercise apart from:

- clock
- hello-world
- react

Add initial content to the user-facing solution file for every other
exercise.

Closes: 409
And for now:

- Implement this outside `check_exercises.nim`.

- Don't optimize by combining the tests for every exercise stub. This
  simplifies checking that running each individual test file produces
  an error when using the stub.
ee7 added 8 commits March 5, 2023 12:30
I'm not convinced that it's worth requiring the user to add this
themselves.
Try to communicate the idea slightly better. We might consider making
both parameters required, but let's leave that for later.

Otherwise, I think it's better to avoid writing `stop = start`, so the
user can discover that for themselves, or be mentored on that point.
@ee7 ee7 marked this pull request as ready for review March 5, 2023 17:31
@ee7 ee7 merged commit 06572c0 into exercism:main Mar 5, 2023
@ee7 ee7 deleted the stubs branch March 5, 2023 17:34
@ee7 ee7 changed the title exercises: stub out every user-facing solution file exercises: provide stubs for every exercise Mar 5, 2023
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.

ci: test that stub files compile, but don't pass tests exercises: expand stub files

2 participants