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

feat: use ast-based helpers #53125

Merged
merged 4 commits into from
Feb 28, 2024

Conversation

ojeytonwilliams
Copy link
Contributor

@ojeytonwilliams ojeytonwilliams commented Jan 11, 2024

Checklist:

This makes use of the new tools for evaluating python code, recently added to https://github.com/freeCodeCamp/curriculum-helpers/ and documented
here

I added a sample curriculum test and there are a bunch of tests of the code itself in the curriculum-helpers. They demonstrate multiple ways to use the library.

@ojeytonwilliams ojeytonwilliams requested a review from a team as a code owner January 11, 2024 14:33
@github-actions github-actions bot added scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. scope: tools/scripts Scripts for supporting dev work, generating config and build artifacts, etc. labels Jan 11, 2024
@ghost
Copy link

ghost commented Jan 11, 2024

👀 Review this PR in a CodeSee Review Map

View the CodeSee Map of this change

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@ojeytonwilliams
Copy link
Contributor Author

I'm not sure why the Italian and Portuguese are failing here, but not in main. The next challenge (step 37) is missing, but that's true for main, too.

I'll dig into that, but otherwise this should be good to go.

Copy link
Member

@ShaunSHamilton ShaunSHamilton left a comment

Choose a reason for hiding this comment

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

Makes sense, but I thought we wanted to move away from using the fake filesystem because it was messing up tests?

@ojeytonwilliams
Copy link
Contributor Author

Makes sense, but I thought we wanted to move away from using the fake filesystem because it was messing up tests?

No, not really. It's more that we want to avoid nesting a test runner inside our own test runner. We didn't need to do it and it make it hard to debug failing tests.

As for why I used it here: it seemed easier than figuring out how to escape the code string. If there's a nice way to do that, we can drop the FS stuff.

Copy link

socket-security bot commented Feb 15, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@freecodecamp/[email protected] None 0 119 kB abdolsa, freecodecamp-team, mot01, ...5 more

🚮 Removed packages: npm/@freecodecamp/[email protected]

View full report↗︎

@ojeytonwilliams
Copy link
Contributor Author

Ooof, more regex issues. @ShaunSHamilton if you've got any ideas, let me know, otherwise I'll have another go.

@ShaunSHamilton
Copy link
Member

@ojeytonwilliams Why is just the i18n stuff failing?

@ojeytonwilliams
Copy link
Contributor Author

@ojeytonwilliams Why is just the i18n stuff failing?

I've not dug into it, but they must have different solutions since they're not syncing properly at the moment.

Assuming that's correct, those solutions are confusing the regex, but are probably valid python.

@ShaunSHamilton
Copy link
Member

This should fix the failing test: freeCodeCamp/curriculum-helpers#148

Here is /english/:

        else:
            
            key_char = key[key_index % len(key)]

Here is /portugues/:

        else:

            key_char = key[key_index % len(key)]

@ojeytonwilliams ojeytonwilliams force-pushed the feat/python-helpers branch 2 times, most recently from b0f36ed to 31dc9e5 Compare February 23, 2024 15:04
@ShaunSHamilton ShaunSHamilton merged commit 630fdac into freeCodeCamp:main Feb 28, 2024
22 checks passed
@ojeytonwilliams ojeytonwilliams deleted the feat/python-helpers branch February 28, 2024 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. scope: tools/scripts Scripts for supporting dev work, generating config and build artifacts, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants