Skip to content
This repository has been archived by the owner on Dec 21, 2023. It is now read-only.

Enabled self-hosted build runners. #10

Merged
merged 7 commits into from Feb 5, 2022
Merged

Conversation

mvastarelli
Copy link
Contributor

Modify build configurations to allow builds to go through our self-hosted build agents.

@mvastarelli mvastarelli added the enhancement New feature or request label Feb 4, 2022
@mvastarelli mvastarelli added this to the 1.0 milestone Feb 4, 2022
@mvastarelli mvastarelli self-assigned this Feb 4, 2022
@mvastarelli
Copy link
Contributor Author

@evan-cohen Can you please take a look at the build logs (https://github.com/bitcobblers/multiminer/actions/runs/1797596167) for this commit? It's now throwing strange linting errors about import/extensions missing a file extension.

Thanks!

@evan-cohen
Copy link
Collaborator

At first glance, it seems like a case sensitivity issue. The file is "Handlers.ts", but the import statement reference "./handlers".

@mvastarelli
Copy link
Contributor Author

At first glance, it seems like a case sensitivity issue. The file is "Handlers.ts", but the import statement reference "./handlers".

Good catch. I forgot that linux was case-sensitive with filenames. Linting is now passing, but now jest is failing with the following error:

1 snapshot file obsolete from 1 test suite. To remove it, run npm test -- -u

Should I just update the build configuration to run tests this way by default?

@evan-cohen
Copy link
Collaborator

I'd typically shy away from appending flags to every run of the test command when it comes to snapshots.

From my reading of the docs, it seems like an "obsolete" snapshot is one for which the original .toMatchSnapshot() can no longer be found (often due to the test being renamed or removed).

That said, the specific file referenced in the error seems to be a false positive due to Jest treating the Linux *.snap file as a snapshot -- possibly a manifestation of jestjs/jest#8922. If that artifact is being generated on every build, we may need to reorder the CI steps to avoid generating that file before running Jest. Far from an ideal solution, but running with -u on every test run would essentially invalidate any snapshot testing that we pursue for the project (as it forces existing snapshots to be ignored and regenerated).

@evan-cohen
Copy link
Collaborator

We might also be able to disable electron-builder's Snap target if we don't intend to distribute to that platform. I believe I remember you mentioning that we're predominantly targeting Windows, right?

@mvastarelli
Copy link
Contributor Author

We might also be able to disable electron-builder's Snap target if we don't intend to distribute to that platform. I believe I remember you mentioning that we're predominantly targeting Windows, right?

I have a suspicion that ERB is doing something strange with the way paths in package.json are resolved. Once I updated the testPathIgnorePatterns using relative paths it was able to exclude the errant snap file.

@mvastarelli
Copy link
Contributor Author

Fixed and fast now.

@mvastarelli mvastarelli merged commit 28179b3 into main Feb 5, 2022
@mvastarelli mvastarelli linked an issue Feb 5, 2022 that may be closed by this pull request
@mvastarelli mvastarelli deleted the br_optimized_builds branch April 10, 2022 20:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimized build configuration.
2 participants