Skip to content

Conversation

@DeanEby
Copy link
Collaborator

@DeanEby DeanEby commented Apr 1, 2025

This PR addresses issue #113. I separated building the executables from uploading the executables and creating a release. This allows us to then test building executables on PR without needlessly uploading artifacts and triggering new releases.
I tested it and was able to run a PR that built without uploading artifacts and was able to trigger a new release with this refactor.
image
image

@DeanEby DeanEby linked an issue Apr 1, 2025 that may be closed by this pull request
@KristijanArmeni
Copy link
Collaborator

KristijanArmeni commented Apr 2, 2025

This looks great @DeanEby ! I guess the only thing we're missing still here is an actual running of the built .exe with the test_build job in test.yaml?

Would, for now, invoking the .exe with a simple --version work? (pseudocode):

  test_build:
      uses: ./.github/workflows/build_exe.yml
      with:
        is_release: false 
      if: runner.os == 'Windows'
        run: |
        .\dist\dist\mangotango.exe --version 

@DeanEby
Copy link
Collaborator Author

DeanEby commented Apr 2, 2025

Good point @KristijanArmeni ! It should be feasible within the build_exe.yml workflow, i'll give it a try.


- name: Check that the executable runs
if: runner.os == 'Windows' && !inputs.is_release
run: ./dist/mangotango.exe --noop
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the build workflow builds and moves the built .exe to a different folder and with a different name?

I see this in l. 19

move_command: move dist\mangotango.exe dist\mangotango_windows.exe

And likewise for MacOS14

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch! Thanks for the tip!

@DeanEby DeanEby merged commit 5b7fa9e into civictechdc:develop Apr 2, 2025
5 checks passed
DeanEby added a commit that referenced this pull request Apr 8, 2025
* Analysisnamingcopy (#111)

* factor: update analysis naming screen copy

* factor: update analysis naming screen copy style

* Prtestbuild [ENH] Add a GitHub action to test .exe upon pull requests #113 (#118)

* refactor: separate building exe and creating new release

* upload artifacts only if triggered from release

* debug: handle workflow inputs

* remove precommit test

* test windows exe run on PR

* debug: fix executable filepath

* debug: fix filepath to updated name

* debug: path / to \

* refactor: implement run checks for macos

* debug: is_release == false instead of !

* Issue #110 new dev workflow documentation (#116)

* Add devops contributor workflow documentation

* Update release section

* feat: 🎸 basic excel importer (#112)

* feat: 🎸 basic excel importer

This Excel importer is very rudimentary but it gets the job done in most
cases. It assumes that the data to be imported lives in one of the
sheets in a "reasonable" position (i.e. headers in Row 1 and first
column is A1). If a file has multiple sheet, the user can change the
sheet to import. No explicit range selection in the sheet itself is
possible.

* run isort for styling

---------

Co-authored-by: DeanEby <ebyd21@gmail.com>

* Opendirectoryscreen copyupdate (#115)

* factor: remove initial output analysis

* refactor: context dependent post-analysis menu

* remove debug print statement

* refactor: slight copy changes

* debug: remove export outputs debug print

* test: 💍 expose CsvConfig for CsvTestData (#114)

* test: 💍 expose CsvConfig for CsvTestData

The CsvConfig class encodes how csv files can be loaded for testing,
but it was never exposed or wired up in the CsvTestData file. This
commit corrects this, and adds an example of how to use it to customize
the way test CSV files are loaded for testing.

* style: 💄 isort && black

---------

Co-authored-by: soul-codes <40335030+soul-codes@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

[ENH] Add a GitHub action to test .exe upon pull requests

3 participants