approvals / ApprovalTests.cpp

Native ApprovalTests for C++ on Linux, Mac and Windows
https://approvaltestscpp.readthedocs.io/en/latest/
Apache License 2.0
310 stars 50 forks source link

Provide a script to allow users to generate the single header from the current code, without doing a release #184

Open asreih opened 3 years ago

asreih commented 3 years ago

Hello,

I have a question on how the single header file is being generated inside the release process, Basically if I only want to generate the single header file, it's including all the headers with their cpps inside "ApprovalTests.cpp/ApprovalTests/ApprovalTests.hpp" when using "create_simulated_single_header_file.py" with the parameter include cpps as true. I am aiming on having the output of the releases where the includes are replaced with the content of the headers/cpp, how can it be done automatically ?

Many thanks in advance for your response, Regards,

claremacrae commented 3 years ago

Hello, thanks for the message.

I’m intrigued to know what problem you are solving - in case it gives ideas for other solutions?

We have a bit of an overview of the release process here ...

https://github.com/approvals/ApprovalTests.cpp/blob/master/build/HowToRelease.md

... essentially it’s a big ball of Python that goes through a lot of steps, including converting that header you’ve been able to generate into a standalone file, that has no references to any headers inside this project.

An end-user wouldn’t be able to easily run our release scripts directly, as there is a whole bunch of other steps that get done, like testing updating of Conan info...

I can’t make any promises about timescales, but I would think that I could extract some of the code out, and create a second script - perhaps called something like “create_standalone_single_header_file.py”

That is already done as part of our Python unit tests - albeit in a test that is often disabled.

asreih commented 3 years ago

Hi Clare Thank you for your swift response, Having a second script to be able to generate the standalone header file would be great, Im interested in knowing which unit test is responsible of that task maybe I can do a PR on that matter if needs be. As for the problem, I find having such script will ease the testing of the header only file and make it easily usable from code wise, not to mention that this can be used inside the release process to make it less coupled since as you said, the release process is tightly coupled. Thank you again for your response

claremacrae commented 3 years ago

Thanks - it's one of the tests that was disabled in this commit: https://github.com/approvals/ApprovalTests.cpp/commit/0031a83af31543c36c416c400283854152de412c#diff-7b4e50b2430c61aacab98e0ac685c041eec58457c5035e68df253a13964ce72e

Probably disabled_test_create_single_header_file_approvals()

Current code:

https://github.com/approvals/ApprovalTests.cpp/blob/master/build/tests/test_for_locking_outputs_from_live_code.py#L16-L17

One issue with making a generic script from this is that the code currently creates a file with a version number in the name...

If we were to have a script that produced the single header from current code, it shouldn't have a version number in the file name.

Current thought: I think the simplest thing would be for the standalone script to write its output to stdout, then callers could redirect it to wherever they wanted to save the output...

PR very welcome!

claremacrae commented 3 years ago

FYI When we change this code, we temporarily re-enable the locking tests and save their output....

Then change the code...

Then re-disable the locking tests...

claremacrae commented 3 years ago

I guess the other thing to work out will be what to put in the version-number section of the output file.

asreih commented 3 years ago

Hello, I suggest the following script to the standalone script "create_standalone_single_header_file.py":

#! /usr/bin/env python3
import os

from scripts.code_generation import CppGeneration
from scripts.project_details import ProjectDetails
from scripts.release_details import ReleaseDetails
from scripts.single_header_file import SingleHeaderFile
from scripts.release_constants import release_constants
from scripts.version import Version

if __name__ == '__main__':
    os.chdir(os.path.join("..", "ApprovalTests"))
    current_version = Version.read(release_constants.build_dir)
    deploy = False
    conan_release = False
    release_details = ReleaseDetails(current_version, current_version, deploy, ProjectDetails(), conan_release)
    cpp_generation = CppGeneration(release_details)
    cpp_generation.prepare_release(release_details)

and for the ReleaseDetails init, it has the additional boolean ", conan_release: bool=True" where it skips the initialization of conan_details.

I had an additional commit to output and return the content of the header file but its not needed since its generating the output under build/releases which is fine.

Let me know if the above approach is fine.

claremacrae commented 3 years ago

Hi, thanks very much for this.

I can confirm that the new script does generate a release file in the build/ directory, which is great...

However, it has the same file-name as the last release, and contains the same version number as the last release...

This is misleading - it will risk confusing users, and will confuse us in user support - when someone says that are using "v.10.9.1" but in fact they are using modified source code, picking up any changes that we made since the last release.

This is why I was preferring that the file was written to standard output, so it didn't need to have a version number in the filename - and the user could decide.

As for the version number inside the contents of the file, I'll have a chat with @isidore and see if we can come up with any ideas... Perhaps including the git id/hash in the version number.

It's really close, and thank you very much for doing this... I'm sure we'll find a way forward...

asreih commented 3 years ago

Hello, IMO, the script can add "-SNAPSHOT" to it to remove the above confusion, As for the output, this is what I done to return the content + print the output optionally, inside code_generation.py: ... def create_single_header_file(self, output_content: bool = False) -> [str,str]: ... if errors != "": raise RuntimeError(errors) if output_content: print(text)

    return os.path.abspath(self.details.release_new_single_header)**,text**

... @staticmethod def prepare_release(details: ReleaseDetails, output_content: bool = False) -> str: code = CppGeneration(details) code.update_version_number_header() new_single_header, text = code.create_single_header_file(output_content) return text

Feel free to choose the way that suits you best, Many thanks for your quick replies

claremacrae commented 3 years ago

Hi @asreih,

Sorry, I don't really understand the formatting of the code in your previous example.

I do like that idea of adding "-snapshot" - if it were possible to add this to the version, then it would be included in both the header and the filename, and work really well.

It doesn't work right now - I tried adding it...

    release_details = ReleaseDetails(current_version, current_version, deploy, ProjectDetails(), conan_release)
+   release_details.new_version.patch += "-snapshot"

... and got this error:

Traceback (most recent call last):
  File "./create_standalone_single_header_file.py", line 17, in <module>
    release_details.new_version.patch += "-snapshot"
TypeError: unsupported operand type(s) for +=: 'int' and 'str'

So the version class would need to be changed to support a 4th field, which is an optional extra string...

I've got a conference talk coming up, so am definitely not going to be able to do this right now...

It sounds to me like you've got something that works for you, for now ... So I'll try to pick this up after the start of July...

Thanks for the suggestion, and the proposed implementation - really helpful, and greatly appreciated!