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.

Sign up for GitHub

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

Jump to bottom

feat(test): run tests and update build result #88

Merged
merged 10 commits into from
Aug 22, 2023
Merged

feat(test): run tests and update build result #88

merged 10 commits into from
Aug 22, 2023

Conversation

JohelEGP
Copy link
Contributor

@JohelEGP JohelEGP commented Jun 6, 2023 •
edited
Loading

  • More field testing.
  • Copy to the repository results that differ from the previous compiler version only.

JohelEGP added 2 commits June 5, 2023 20:23
@JohelEGP
refactor(test): update results as fixture cleanups
ddce5c6 
This is less wasteful as it makes use of the output of existing tests.
It should simplify adding support for running the tests.
@JohelEGP
refactor(test): order build after codegen
1267251 
Ordering by relative cost reduces turnarounds.
Previously, codegen update happened last.
@JohelEGP JohelEGP changed the title Developing tests feat(test): run tests and update build result Jun 6, 2023
@JohelEGP
feat(test): run tests and update build result
ed9d696 
Although the project's language is now `CXX`,
the setup of the C++ environment is as was before.
I.e.,
- The environment of the build test can only be set globally
  (through CMake defaults, the environment, etc.).
- The environment of this project should match that of the build test.
  It doesn't attempt to propagate any aspect explicitly.
@JohelEGP JohelEGP force-pushed the developing_tests branch from ddc1d0e to ed9d696 Compare June 6, 2023 04:55
JohelEGP
JohelEGP commented Jun 6, 2023
View reviewed changes
regression-tests/CMakeLists.txt
Comment on lines +149 to +148
# There's `CMAKE_CXX_LINKER_LAUNCHER`, too. So far, it's not needed.
"-DCMAKE_CXX_COMPILER_LAUNCHER=${CMAKE_COMMAND};-D;OUTPUT_FILE=${gen_cpp_src}.output;-P;../../ExecuteWithRedirection.cmake;--"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These only work with "The Makefile Generators and the Ninja generator".
The alternative is CMAKE_EXPORT_COMPILE_COMMANDS.
I already handle that at https://github.com/JohelEGP/jegp.cmake_modules/blob/master/modules/.detail/JEGPReadExportedCompileCommand.cmake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would still need to build and install.
So I'd compile twice to get the output.
Maybe I can hijack CMAKE_CXX_COMPILER
and use it like CMAKE_CXX_COMPILER_LAUNCHER.

@JohelEGP JohelEGP force-pushed the developing_tests branch from cb7f504 to 65a99b9 Compare June 7, 2023 19:29
@JohelEGP JohelEGP force-pushed the developing_tests branch 3 times, most recently from d54db4d to a74594f Compare June 8, 2023 21:37
@JohelEGP JohelEGP force-pushed the developing_tests branch from a74594f to 369aa23 Compare June 8, 2023 21:48
@JohelEGP JohelEGP marked this pull request as ready for review June 10, 2023 05:29
@JohelEGP
fix(test): add non-check tests when developing
cb0a142 
Test results will be updated,
so make the tests use those results
without a reconfiguration in-between.
Also, trigger reconfiguration to add the `check` tests.
@JohelEGP JohelEGP mentioned this pull request Jun 14, 2023
Open
@JohelEGP
Copy link
Contributor Author

All this would be much simpler
if we could just work in the Git repository itself
rather than on copies of its files.

@alexreinking
Copy link
Contributor

All this would be much simpler if we could just work in the Git repository itself rather than on copies of its files.

I agree, but @hsutter didn't want to include a CMake build in the upstream cppfront, at least not yet. See the discussion here: hsutter/cppfront#15 (comment)

It has been some time now, though -- perhaps Herb would be willing to reconsider?

@alexreinking
Copy link
Contributor

Also, is this ready to review?

@JohelEGP
Copy link
Contributor Author

Also, is this ready to review?

Yes, since #88 (comment).
I still continue to find rough edges when developing.

All this would be much simpler if we could just work in the Git repository itself rather than on copies of its files.

I agree, but @hsutter didn't want to include a CMake build in the upstream cppfront, at least not yet. See the discussion here: hsutter/cppfront#15 (comment)

It has been some time now, though -- perhaps Herb would be willing to reconsider?

I was hoping we wouldn't be blocked on that.

I've also found the need to manually specify the tests' build type as Debug because it's hard-coded.
So it would be much better if there was a single CMake project,
even if we can't have subdirectories within the cppfront git submodule.

@alexreinking alexreinking self-requested a review July 10, 2023 19:54
@alexreinking
Copy link
Contributor

Hey, sorry I took so long to get back to this -- merging now...

@alexreinking alexreinking merged commit 472e744 into modern-cmake:main Aug 22, 2023
@JohelEGP JohelEGP deleted the developing_tests branch August 22, 2023 16:00
@JohelEGP JohelEGP restored the developing_tests branch August 22, 2023 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexreinking alexreinking Awaiting requested review from alexreinking

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JohelEGP @alexreinking