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: e2e getStack, disable failing e2e #5768

Merged
merged 9 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/ci-setup-action/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,5 +64,5 @@ runs:
echo "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" > "/run/${{ inputs.concurrency_key }}.lock"
echo "/run/${{ inputs.concurrency_key }}.lock acquired."
post: |
rm "/run/${{ inputs.concurrency_key }}.lock"
rm "/run/${{ inputs.concurrency_key }}.lock" || true
echo "/run/${{ inputs.concurrency_key }}.lock removed."
1 change: 0 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ jobs:
timeout-minutes: 40
run: earthly ./yarn-project+export-end-to-end
# We base our e2e list used in e2e-x86 off the targets in ./yarn-project/end-to-end
# (Note ARM uses just 2 tests as a smoketest)
- name: Create list of end-to-end jobs
id: e2e_list
run: echo "list=$(earthly ls ./yarn-project/end-to-end | grep -v '+base' | sed 's/+//' | jq -R . | jq -cs .)" >> $GITHUB_OUTPUT
Expand Down
6 changes: 4 additions & 2 deletions yarn-project/circuit-types/src/simulation_error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ export class SimulationError extends Error {
options?: ErrorOptions,
) {
super(originalMessage, options);
const getMessage = () => this.getMessage();
const getStack = () => this.getStack();
Object.defineProperties(this, {
message: {
configurable: false,
Expand All @@ -82,7 +84,7 @@ export class SimulationError extends Error {
* @returns The message.
*/
get() {
return this.getMessage();
return getMessage();
},
},
stack: {
Expand All @@ -93,7 +95,7 @@ export class SimulationError extends Error {
* @returns The stack.
*/
get() {
return this.getStack();
Copy link
Contributor

Choose a reason for hiding this comment

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

👍
Still curios to find out why getStack is undefined here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The this here is just wrong! it's the this of the stack object, not class

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You can tell because it has type any in TS on hover in IDE

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I don't think that's it. The spec says this is going to be the object the property is accessed on. This works as expected:

let obj = { getStack() { return "foo"; } }
undefined
Object.defineProperty(obj, "stack", {
  get() {
    return this.getStack();
  }
});
Object { getStack: getStack(),}

obj.stack 
"foo" 

return getStack();
},
},
});
Expand Down
Loading