Skip to content

Conversation

@palimondo
Copy link
Contributor

@palimondo palimondo commented Nov 17, 2018

Two small fixes:

  • use SWIFT_DETERMINISTIC_HASHING
  • allow running tests with dot and dash in the name

The naming change is to allow for measuring tests conforming to proposed naming convention in #20334 (only partial support for . and -, for now without ! and ?), like the #20552 and #20666.

If these get accepted, the Apple-internal script that measures benchmarks and submits them to LNT should also be updated to work with such names.

Tests that used hashing were being unnecessarily tested multiple times, because this environment variable was missing.
@palimondo
Copy link
Contributor Author

@eeckstein Please review 🙏

Allow for running of test matching the naming convention proposed in swiftlang#20334.
@eeckstein
Copy link
Contributor

@swift-ci benchmark

1 similar comment
@eeckstein
Copy link
Contributor

@swift-ci benchmark

@eeckstein eeckstein self-requested a review November 26, 2018 17:50
Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

lgtm

@eeckstein
Copy link
Contributor

@swift-ci smoke test

1 similar comment
@eeckstein
Copy link
Contributor

@swift-ci smoke test

@swift-ci
Copy link
Contributor

Build comment file:

No performance and code size changes


@eeckstein eeckstein merged commit 5a6dfb6 into swiftlang:master Nov 26, 2018
@palimondo
Copy link
Contributor Author

Thank you Erik!

@palimondo
Copy link
Contributor Author

@eeckstein just quick a reminder from above, as it looks like #20769 also uses periods in names:

If these get accepted, the Apple-internal script that measures benchmarks and submits them to LNT should also be updated to work with such names.

@lorentey
Copy link
Member

If the dots cause any trouble, I can still change them!

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.

4 participants