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

fix(test): unifying dan layer integration tests #4175

Merged
merged 10 commits into from
Jun 9, 2022

Conversation

mrnaveira
Copy link
Contributor

Description

  • Solving cucumber tag duplication of @dan_layer and @dan tags, used only the @dan tag in all related tests
  • Mark as broken all old tests for the DAN layer, they do not pass anymore
  • Mark as broken the recent contract acceptance integration test. Some recent change did seem to broke it, it will be fixed in a separate PR in the future
  • For name consistency, renaming the term "constitution definition" for the more appropriate "contract constitution"
  • Adapted the wallet CLI tests for contract definition and constitution to the new CLI (the --command option is no longer valid)
  • Tagged the working DAN layer tests as @critical, to run them on new PR submissions to avoid breaking changes

Motivation and Context

It's desirable to be able to run all DAN related integration tests together under the same tag, and know if we are breaking existing functionalities when developing new features in the future.

How Has This Been Tested?

Now, the cucumber integration tests ran with --tags "@dan and not @broken" pass

therustmonk
therustmonk previously approved these changes Jun 8, 2022
Copy link
Contributor

@therustmonk therustmonk left a comment

Choose a reason for hiding this comment

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

LGTM

sdbondi
sdbondi previously approved these changes Jun 9, 2022
Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

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

Thanks LGTM - Ack (tested in CI)

Approving because comments are minor / all tests passing. We reapprove if suggested changes are made.

integration_tests/features/WalletCli.feature Show resolved Hide resolved
integration_tests/helpers/walletProcess.js Outdated Show resolved Hide resolved
@mrnaveira mrnaveira dismissed stale reviews from sdbondi and therustmonk via 5f509f2 June 9, 2022 06:51
mrnaveira and others added 2 commits June 9, 2022 07:51
Co-authored-by: Stan Bondi <sdbondi@users.noreply.github.com>
@aviator-app aviator-app bot merged commit f3495ee into tari-project:development Jun 9, 2022
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.

3 participants