GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — test.cpp: also run tests with char buffer by firewave · Pull Request #261 · cppcheck-opensource/simplecpp

Conversation

@firewave

@firewave firewave commented Apr 20, 2022

Copy link
Copy Markdown
Collaborator

No description provided.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@firewave firewave changed the title test.cpp: run tests with std::istringstream and std::ifstream run tests with std::istringstream and std::ifstream Apr 20, 2022
@firewave firewave changed the title run tests with std::istringstream and std::ifstream run tests with std::istringstream and std::ifstream Apr 20, 2022
danmar
danmar reviewed Apr 20, 2022
View reviewed changes
Comment thread test.cpp Outdated

#define TEST_CASE(F) (testcase(#F, F, argc, argv))

static std::string writeFile(const char code[], std::size_t size, const std::string &filename) {

@danmar danmar Apr 20, 2022

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I feel that it's a good idea to avoid using real files in tests.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@firewave firewave Apr 21, 2022

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

In general I totally agree with you. But we allow the input of actual files so we should actually test this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@danmar danmar Apr 21, 2022

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I still feel skeptic. if we assume that the stream input is bug free then the test should work the same with a string input or a file. if we extrapolate this then this should be done in cppcheck also. I fear I don't like this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@firewave firewave Aug 23, 2022

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry for the late reply.

Keeping tests simple is great but I think there should be proper coverage and historically that is something we don't have in either Cppcheck or simplecpp. 😐

Also all the include files are always being read from the disk and not from memory. Not having any tests which rely on temporary files indicates that there is no coverage for reading includes at all. I have not looked into this yet.

So it seems instead of avoiding tests with files on disk we actually need to add much more.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@danmar danmar Aug 23, 2022

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Having some system test that uses files is good but we do have some such system tests. Testing that APIs work as they should is out of scope.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@firewave firewave Oct 6, 2022

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Without this I am not able to test the changes in #244. If that is fine with you I will try to finish up that PR without any unit tests.

I will prepare another PR to highlight with more tests to highlight the underlying testing issue.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@danmar danmar Oct 6, 2022

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If that is fine with you I will try to finish up that PR without any unit tests.

yes I would prefer that in this case.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@firewave firewave Oct 20, 2022

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Great.

After thinking about it a bit more the usage with Cppcheck could be considered an integration test.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@danmar danmar Oct 22, 2022

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

yes it's more like a integration test.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@firewave
firewave marked this pull request as draft October 20, 2022 12:58
@firewave firewave mentioned this pull request Oct 27, 2022
added TokenList::Stream class to wrap std::istream usage and implemented alternative C I/O version #244
Merged
@firewave firewave mentioned this pull request Dec 24, 2022
Includes of system headers are never implicitly relative to the source file #282
Merged
@firewave
firewave force-pushed the test-x branch from 5069892 to 7a89605 Compare December 29, 2022 15:35
@firewave
firewave force-pushed the test-x branch from 7a89605 to 16f2269 Compare March 3, 2023 09:00
@firewave
firewave force-pushed the test-x branch from 4acb91b to 1dc1258 Compare March 14, 2023 21:18
@firewave firewave mentioned this pull request Mar 21, 2023
added asserts to streams to make sure the input is valid #294
Merged
@firewave firewave changed the title run tests with std::istringstream and std::ifstream run tests with std::istringstream,std::ifstream and file input Mar 21, 2023
@firewave
firewave force-pushed the test-x branch from 28e22ba to 9de10ee Compare September 26, 2023 06:44
@firewave firewave mentioned this pull request Mar 27, 2024
made it possible to create a TokenList from a buffer #347
Merged
@firewave
firewave force-pushed the test-x branch from 9de10ee to a483a1d Compare March 28, 2024 08:53
@firewave firewave changed the title run tests with std::istringstream,std::ifstream and file input run tests with std::istringstream,std::ifstream,std::string and file input Mar 28, 2024
@firewave
firewave force-pushed the test-x branch 2 times, most recently from 5d99779 to b196f5b Compare March 29, 2024 18:13
@firewave firewave changed the title run tests with std::istringstream,std::ifstream,std::string and file input run tests with std::istringstream,std::ifstream, buffer and file input Mar 29, 2024
@firewave
firewave force-pushed the test-x branch from 99cc8fb to b196f5b Compare March 29, 2024 18:31
@firewave firewave mentioned this pull request Aug 6, 2024
fix: use both absolute and relative header paths in header matching #362
Merged
@firewave firewave mentioned this pull request Aug 19, 2025
added TokenList constructors with modern buffer wrappers and hide "unsafe" ones - if available #496
Merged
@firewave

firewave commented Sep 10, 2025

Copy link
Copy Markdown
Collaborator Author

This could be implemented in the Python tests instead by making this modes available via simplecpp (as already is via -is). That would require to parameterize each invocation but I am not sure how to do this. @Tal500 any idea?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@firewave

firewave commented Oct 11, 2025

Copy link
Copy Markdown
Collaborator Author

Thinking about it I think this is still valid except for the file input.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@firewave firewave mentioned this pull request Jan 15, 2026
test.cpp: improved testing of __FILE__ #623
Merged
@firewave
firewave force-pushed the test-x branch 4 times, most recently from 929222a to 07a2beb Compare January 15, 2026 10:09
@firewave firewave changed the title run tests with std::istringstream,std::ifstream, buffer and file input test.cpp: also run tests with char buffer Jan 15, 2026
@firewave
firewave force-pushed the test-x branch from 07a2beb to 7b666de Compare January 15, 2026 10:10
@firewave

firewave commented Jan 15, 2026

Copy link
Copy Markdown
Collaborator Author

I removed everything which generates a file.

I will add flags to the CLI to be able to use those in the Python tests but that won't have the same coverage as using it in the unit tests and might detect some issues only as in drive-by way (as experienced in #566).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@firewave
firewave force-pushed the test-x branch 4 times, most recently from 3db7f08 to 62d73ff Compare January 15, 2026 10:29
@firewave firewave mentioned this pull request Jan 16, 2026
run Python tests with all possible Stream types #625
Draft
@Tal500

Tal500 commented Jan 19, 2026

Copy link
Copy Markdown
Contributor

available

sorry for the very late reply. see https://github.com/danmar/simplecpp/blob/10c96815e402299da1d3ab1f753dda4c1b0b12f8/integration_test.py#L37-L39
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@firewave

firewave commented Jan 19, 2026

Copy link
Copy Markdown
Collaborator Author

sorry for the very late reply. see

No problem at all. Thanks for replying.

https://github.com/danmar/simplecpp/blob/10c96815e402299da1d3ab1f753dda4c1b0b12f8/integration_test.py#L37-L39

I do get have parameterizing works but this requires every simplecpp call to gain an additional parameter so this needs needs something global. Possibly requiring a fixture (or whatever it is called in Python).

BTW I moved that to #625.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@firewave
firewave force-pushed the test-x branch from 62d73ff to 1c398d9 Compare February 10, 2026 20:50
@firewave firewave mentioned this pull request Mar 27, 2026
Hang on UTF-16 with BOM #637
Closed
@firewave
test.cpp: also run tests with char buffer
5a85b00
@firewave
firewave force-pushed the test-x branch from 612a069 to 5a85b00 Compare April 13, 2026 11:10
firewave added 2 commits April 13, 2026 13:25
@firewave
test.cpp: fixed misc-use-internal-linkage clang-tidy warning
e6c4acf
@firewave
test.cpp: mitigated compiler warnings about missing return with GCC a…
7c1526b 
…nd Visual Studio
@firewave
firewave force-pushed the test-x branch from 525bed0 to 7c1526b Compare April 13, 2026 11:29
@firewave
firewave marked this pull request as ready for review April 13, 2026 11:36
danmar
danmar approved these changes Apr 16, 2026
View reviewed changes
@firewave
firewave merged commit 316d4ee into cppcheck-opensource:master Apr 16, 2026
24 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@firewave
firewave deleted the test-x branch April 16, 2026 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danmar danmar danmar approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@firewave @Tal500 @danmar
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.