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

Parser will fail if the name of the test is split over several lines in the test file. #73

Open
vaeng opened this issue Aug 15, 2023 · 7 comments

Comments

@vaeng
Copy link
Contributor

vaeng commented Aug 15, 2023

No description provided.

@ahans
Copy link

ahans commented Mar 15, 2024

I think we can fix the multiline issue by replacing the find with something more powerful like a regex. However, I think it'd be better to use something that actually understands C++. I have used libclang a couple of years ago for some C++ parsing and it worked fairly well. I would give it a shot here, unless it's a dependency you wouldn't want to introduce.

Also, I wonder why this is even written in C++? I personally would prefer Python for tasks like this. But I don't know enough about the test runner yet to say if a rewrite in Python would make sense.

@siebenschlaefer
Copy link

I'm fine with any tech-stack that gives us a working solution and is easy to maintain.
But the larger the Docker image gets the more Exercism has to pay.
I think that's why @vaeng tried to keep it as small as possible.

@vaeng
Copy link
Contributor Author

vaeng commented Mar 15, 2024

Siebenschläfer is right, the initial thought was to get rid of the Pythond dependency to reduce the footprint of the test-runner. Every gig is about 1k€ per year.

Also, the general directive from Erik was that the runner should be written in the same language as the track.

I am very happy to see a more versatile solution. If you get some hold of a nice ast parser for Cpp, we can use it to make the represented a lot better and even implement an analyzer.

@ahans
Copy link

ahans commented Mar 15, 2024

Siebenschläfer is right, the initial thought was to get rid of the Pythond dependency to reduce the footprint of the test-runner.

Turns out Python is already part of the image:

cpp-test-runner git:main  
❯ docker run -ti --rm --entrypoint python exercism/test-runner
Python 3.11.8 (main, Feb 19 2024, 22:58:08) [GCC 12.2.1 20220924] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> 

Looks like it's pulled by boost-dev through boost-python.

Also, the general directive from Erik was that the runner should be written in the same language as the track.

How hard of a requirement is that? I find it goes a bit against the "right tool for the job" best practice.

I am very happy to see a more versatile solution. If you get some hold of a nice ast parser for Cpp, we can use it to make the represented a lot better and even implement an analyzer.

I think libclang would be a good fit here. It would also not increase the size of the image, since it would only be a build-time requirement. The final executable would be a few kilobytes larger, but that'd be it.

@vaeng
Copy link
Contributor Author

vaeng commented Mar 15, 2024

It's not a hard requirement.

@ahans
Copy link

ahans commented Mar 15, 2024

How much of a requirement is boost? I know that at least date_time is used in one exercise. Anything else? Are there many implementations by students that rely on other boost libraries? If image size is really a concern, we could probably shave off 200 MB by manually installing only what we need.

@vaeng
Copy link
Contributor Author

vaeng commented Mar 15, 2024

I'd say it's required that all existing student solutions are still working after changes in the test runner. So I would give the boost lib a hard yes for requirements.

ahans added a commit to ahans/cpp that referenced this issue Apr 3, 2024
Some tests get manual formatting and local `clang-format off` to work around
exercism/cpp-test-runner#73
ahans added a commit to ahans/cpp that referenced this issue Apr 9, 2024
Some tests get manual formatting and local `clang-format off` to work around
exercism/cpp-test-runner#73
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

No branches or pull requests

3 participants