Skip to content

feat(models): add report filename option - #174

Merged
BioPhoton merged 15 commits into
mainfrom
add-report-filename-option
Oct 30, 2023
Merged

feat(models): add report filename option#174
BioPhoton merged 15 commits into
mainfrom
add-report-filename-option

Conversation

@BioPhoton

@BioPhotonBioPhoton commented Oct 28, 2023

Copy link
Copy Markdown
Collaborator

The changes should help to improve testing of report generation. ATM all tests write in the same folder and with the same file name. Adding a filename option to core would improve the testing situation and is helpful in for integrations.

This PR includes:

Implementing the CLI option is resolved in a followup PR

closes#161

@BioPhotonBioPhoton mentioned this pull request Oct 28, 2023
2 tasks
@BioPhotonBioPhoton added ➕ enhancement new feature or request 🔬 testing writing tests 🧩 core labels Oct 29, 2023
Comment threadpackages/models/src/lib/implementation/utils.spec.ts Outdated
Comment threadpackages/models/src/lib/implementation/utils.spec.ts Outdated
Comment threadpackages/models/src/lib/implementation/schemas.ts Outdated
Comment threade2e/cli-e2e/tests/print-config.spec.ts Outdated
Comment threade2e/cli-e2e/tests/print-config.spec.ts Outdated
Comment threadpackages/models/test/fixtures/report.mock.ts
BioPhotonand others added 5 commits October 29, 2023 18:31
Co-authored-by: Matěj Chalk <34691111+matejchalk@users.noreply.github.com>
Co-authored-by: Matěj Chalk <34691111+matejchalk@users.noreply.github.com>
Co-authored-by: Matěj Chalk <34691111+matejchalk@users.noreply.github.com>
Co-authored-by: Matěj Chalk <34691111+matejchalk@users.noreply.github.com>
Co-authored-by: Matěj Chalk <34691111+matejchalk@users.noreply.github.com>
@BioPhotonBioPhoton self-assigned this Oct 29, 2023

@matejchalkmatejchalk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It works nicely with the config 👍

The PR description is a bit misleading, --filename (or --persist.filename) doesn't work as a CLI argument. You'd have to add persist.filename to core-config-options.ts to make it accessible from the CLI (we should auto-sync those in the future, probably).

Also, I noticed the PR title doesn't follow our commit conventions - model scope should be models.

matejchalk
matejchalk previously approved these changes Oct 30, 2023
vmasek
vmasek previously approved these changes Oct 30, 2023
@BioPhoton
BioPhoton dismissed stale reviews from vmasek and matejchalk via db2c06dOctober 30, 2023 11:43
@BioPhotonBioPhoton changed the title feat(model): add report filename optionrefactor(model): add report filename optionOct 30, 2023
@BioPhotonBioPhoton changed the title refactor(model): add report filename optionfeat(models): add report filename optionOct 30, 2023
@BioPhoton
BioPhoton merged commit bdeab54 into mainOct 30, 2023
@BioPhoton
BioPhoton deleted the add-report-filename-option branch October 30, 2023 21:02
@kristihoskovakristihoskova mentioned this pull request Nov 13, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🧩 core➕ enhancementnew feature or request🔬 testingwriting tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add optional filename to PersistConfig

3 participants

@BioPhoton@vmasek@matejchalk