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.

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

feat: add gnodev test -update-golden-tests flag #449

Merged
merged 10 commits into from
Feb 18, 2023

Conversation

piux2
Copy link
Contributor

@piux2 piux2 commented Jan 5, 2023

Description

#440 3 of 3

This commit is the last step to fixing the false testing result during Github CI
We keep the feature to sync the actual output to test comments in files. It allowed us to see wanted outcomes overwritten by actual results during local testing. "Git status" shows modified testing files.

  1. Added a syncWanted parameter to the RunFileTest() in tests/file.go and runFileTest() in tests/file_test.go. It makes the function contract clear without relying upon the side effect of the syncWanted variable while we still maintain the custom testing flag available in file_test.go for the Makefile to execute in CI.

  2. Set package variable syncWanted as a golang custom testing flag --sync in tests/file_test.go. The default value is false. It allows the CI to stop at failure cases when executing file tests in the tests/files and tests/files2 folder

  3. Added --sync flog in cmd/gnodev/test.go. The default value is false. It allows CI to stop at failure cases when executing file tests in ./examples

  4. Added three entries in the Makefile. It allows us to turn on the sync flag during the local testing
    test.files1.sync
    test.files2.sync
    test.examples.sync

  5. To run tests/files and tests/files2 file test with sync on

    go test ./tests -v --sync
    
  6. To run file test in ./examples with sync on

    ./build/gnodev test ./examples --sync
    

How has this been tested?

  1. Modify the realm hash in

    tests/files/zrealm_example.gno
    tests/files2/zrealm_example.gno

    make test.files1 - failed
    make test.files2 - failed

    make test.files1.sync - passed
    make test.files2.sync - passed

    The correct hash value was written back to the file test comments.

  2. Replaced print with println in examples/gno.land/r/demo/releases-example/release0_filetest.gno

    make test.examples - failed

    make test.examples.sync - passed

    The correct rendering output was written back to the file test comments.

Copy link
Member

@zivkovicmilos zivkovicmilos left a comment

Choose a reason for hiding this comment

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

Looks great 💯

Thank you for the contribution as always 🙏

@zivkovicmilos zivkovicmilos requested a review from a team January 16, 2023 15:10
Makefile Outdated Show resolved Hide resolved
Copy link
Member

@moul moul left a comment

Choose a reason for hiding this comment

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

thank you very much for this contrib. However, I think sync is too ambiguous and risks conflicting with a potential upcoming feature.

what do you think about renaming --sync to --update-golden-tests?

@piux2
Copy link
Contributor Author

piux2 commented Jan 20, 2023

thank you very much for this contrib. However, I think sync is too ambiguous and risks conflicting with a potential upcoming feature.

what do you think about renaming --sync to --update-golden-tests?

Sure, that makes sense. I will update it.

@piux2 piux2 closed this Jan 20, 2023
@piux2 piux2 reopened this Jan 20, 2023
@piux2 piux2 requested a review from moul January 20, 2023 21:35
@piux2
Copy link
Contributor Author

piux2 commented Feb 18, 2023

thank you very much for this contrib. However, I think sync is too ambiguous and risks conflicting with a potential upcoming feature.

what do you think about renaming --sync to --update-golden-tests?

@moul can you approve this?

@moul moul changed the title Fix: file tests should have failed in Github CI. Added sync flag to sync the wanted and actual results for local testing feat: add gnodev test -update-golden-tests flag Feb 18, 2023
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
@moul moul self-assigned this Feb 18, 2023
@moul moul merged commit 1d36508 into gnolang:master Feb 18, 2023
@moul moul added this to the 🌟 main.gno.land (wanted) milestone Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants