exercism / coffeescript

Exercism exercises in CoffeeScript.
https://exercism.org/tracks/coffeescript
MIT License
19 stars 50 forks source link

fix pangram.spec.coffee #274

Closed ladokp closed 4 months ago

ladokp commented 4 months ago

@BNAndras, @ErikSchierboom: Since the tests are preventing a submition via online editor, this is a shot in the blue since this strings are the only odd thing I can find in this file.

See: https://github.com/exercism/coffeescript/pull/269#issuecomment-1945922487

github-actions[bot] commented 4 months ago

Hello. Thanks for opening a PR on Exercism 🙂

We ask that all changes to Exercism are discussed on our Community Forum before being opened on GitHub. To enforce this, we automatically close all PRs that are submitted. That doesn't mean your PR is rejected but that we want the initial discussion about it to happen on our forum where a wide range of key contributors across the Exercism ecosystem can weigh in.

You can use this link to copy this into a new topic on the forum. If we decide the PR is appropriate, we'll reopen it and continue with it, so please don't delete your local branch.

If you're interested in learning more about this auto-responder, please read this blog post.


Note: If this PR has been pre-approved, please link back to this PR on the forum thread and a maintainer or staff member will reopen it.

BNAndras commented 4 months ago

It’s strange because I don’t see that error in the CI. Hopefully the test runner isn’t doing something differently.

ladokp commented 4 months ago

Yes, it's really strange. But this is the only oddity I found in the file. I'm just curious if the tests are successful when doing this "fix"...

Especially because the output states that the tests are successful indeed but something tells the online editor that it isn't.

ladokp commented 4 months ago

I've seen this before: https://github.com/exercism/coffeescript/pull/259#issuecomment-1919050387 Succesfull merge and CI checks but no possibility to submit.

BNAndras commented 4 months ago

The CI runs npm install -g jasmine-node coffeescript before testing exercises. The test runner has pinned dependencies at https://github.com/exercism/coffeescript-test-runner/blob/main/package-lock.json. That might mean there's a versioning issue here.

I'd say we update the CI to use the test runner image to test the exercises. That should nip the issue in the bud.

ladokp commented 4 months ago

Sounds good. 😀

ladokp commented 4 months ago

Would be great if the CI and the test runner are working the same way.

ladokp commented 4 months ago

@BNAndras, I looked at the two mentioned packages. The current versions are exactly that ones the test runner is pinned to. Therefore I think that the CI should currently also install exactly this versions when nothing is pinned.

ladokp commented 4 months ago

@ErikSchierboom: May we test if my (maybe nonsense) string change makes a difference?

ErikSchierboom commented 4 months ago

It does fix the issue. The problem is not the actual test output, which is fine even in the old situation:

<?xml version="1.0" encoding="UTF-8" ?>
<testsuites>
<testsuite name="Pangram" errors="0" tests="10" failures="0" time="0.004" timestamp="2024-02-16T08:43:18">
  <testcase classname="Pangram" name="empty sentence" time="0.002"></testcase>
  <testcase classname="Pangram" name="perfect lower case" time="0"></testcase>
  <testcase classname="Pangram" name="only lower case" time="0"></testcase>
  <testcase classname="Pangram" name="missing the letter &amp;quot;x&amp;quot;" time="0"></testcase>
  <testcase classname="Pangram" name="missing the letter &amp;quot;h&amp;quot;" time="0"></testcase>
  <testcase classname="Pangram" name="with underscores" time="0.001"></testcase>
  <testcase classname="Pangram" name="with numbers" time="0"></testcase>
  <testcase classname="Pangram" name="missing letters replaced by numbers" time="0"></testcase>
  <testcase classname="Pangram" name="mixed case and punctuation" time="0"></testcase>
  <testcase classname="Pangram" name="a-m and A-M are 26 different characters but not a pangram" time="0"></testcase>
</testsuite>
</testsuites>

I think it likely has to do with using the ast.json file to extract the test code. Worth fixing in the test runner if would say.

p.s. I can't re-open this PR as it was force pushed or recreated,

ladokp commented 4 months ago

Oh, I rebased my branch. Will create a new PR, sorry.

ladokp commented 4 months ago

@ErikSchierboom: created #275